mirror of
https://github.com/frappe/erpnext.git
synced 2026-04-24 17:18:30 +00:00
fix: correct logic for repair cost in asset repair
This commit is contained in:
@@ -111,15 +111,13 @@ frappe.ui.form.on("Asset Repair", {
|
|||||||
purchase_invoice: function (frm) {
|
purchase_invoice: function (frm) {
|
||||||
if (frm.doc.purchase_invoice) {
|
if (frm.doc.purchase_invoice) {
|
||||||
frappe.call({
|
frappe.call({
|
||||||
method: "frappe.client.get_value",
|
method: "erpnext.assets.doctype.asset_repair.asset_repair.get_repair_cost_for_purchase_invoice",
|
||||||
args: {
|
args: {
|
||||||
doctype: "Purchase Invoice",
|
purchase_invoice: frm.doc.purchase_invoice,
|
||||||
fieldname: "base_net_total",
|
|
||||||
filters: { name: frm.doc.purchase_invoice },
|
|
||||||
},
|
},
|
||||||
callback: function (r) {
|
callback: function (r) {
|
||||||
if (r.message) {
|
if (r.message) {
|
||||||
frm.set_value("repair_cost", r.message.base_net_total);
|
frm.set_value("repair_cost", r.message);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -3,6 +3,7 @@
|
|||||||
|
|
||||||
import frappe
|
import frappe
|
||||||
from frappe import _
|
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
|
from frappe.utils import add_months, cint, flt, get_link_to_form, getdate, time_diff_in_hours
|
||||||
|
|
||||||
import erpnext
|
import erpnext
|
||||||
@@ -308,9 +309,14 @@ class AssetRepair(AccountsController):
|
|||||||
if flt(self.repair_cost) <= 0:
|
if flt(self.repair_cost) <= 0:
|
||||||
return
|
return
|
||||||
|
|
||||||
pi_expense_account = (
|
expense_accounts = _get_expense_accounts_for_purchase_invoice(self.purchase_invoice)
|
||||||
frappe.get_doc("Purchase Invoice", self.purchase_invoice).items[0].expense_account
|
|
||||||
)
|
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(
|
gl_entries.append(
|
||||||
self.get_gl_dict(
|
self.get_gl_dict(
|
||||||
@@ -473,3 +479,82 @@ class AssetRepair(AccountsController):
|
|||||||
def get_downtime(failure_date, completion_date):
|
def get_downtime(failure_date, completion_date):
|
||||||
downtime = time_diff_in_hours(completion_date, failure_date)
|
downtime = time_diff_in_hours(completion_date, failure_date)
|
||||||
return round(downtime, 2)
|
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
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ from frappe import qb
|
|||||||
from frappe.query_builder.functions import Sum
|
from frappe.query_builder.functions import Sum
|
||||||
from frappe.utils import add_days, add_months, flt, get_first_day, nowdate, nowtime, today
|
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 (
|
from erpnext.assets.doctype.asset.asset import (
|
||||||
get_asset_account,
|
get_asset_account,
|
||||||
get_asset_value_after_depreciation,
|
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 (
|
from erpnext.assets.doctype.asset_depreciation_schedule.asset_depreciation_schedule import (
|
||||||
get_asset_depr_schedule_doc,
|
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.item.test_item import create_item
|
||||||
from erpnext.stock.doctype.serial_and_batch_bundle.test_serial_and_batch_bundle import (
|
from erpnext.stock.doctype.serial_and_batch_bundle.test_serial_and_batch_bundle import (
|
||||||
get_serial_nos_from_bundle,
|
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(asset.additional_asset_cost, asset_repair.repair_cost)
|
||||||
self.assertEqual(booked_value, 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):
|
def num_of_depreciations(asset):
|
||||||
return asset.finance_books[0].total_number_of_depreciations
|
return asset.finance_books[0].total_number_of_depreciations
|
||||||
|
|||||||
Reference in New Issue
Block a user