mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-12 06:01:46 +00:00
refactor(sales_invoice): simplify fixed-asset and inter-company validations
validate_fixed_asset (C/11) is flattened with a guard clause and the per-item checks move into _validate_fixed_asset_item. validate_inter_company_party (C/12) splits into _get_inter_company_party_config plus _validate_against_reference and _validate_internal_party_company (conditions preserved verbatim). No C-rank function remains in either module. Characterize the previously-untested asset-sale throws (missing asset, update stock, return without return-against, selling a sold/scrapped asset) and the asset-restore note text before the move; behaviour is unchanged (asset and inter-company suites green).
This commit is contained in:
@@ -25,30 +25,35 @@ class FixedAssetService:
|
||||
if doc.doctype != "Sales Invoice":
|
||||
return
|
||||
|
||||
for d in doc.get("items"):
|
||||
if not d.is_fixed_asset:
|
||||
continue
|
||||
for item in doc.get("items"):
|
||||
if item.is_fixed_asset:
|
||||
self._validate_fixed_asset_item(item)
|
||||
|
||||
if d.asset:
|
||||
if not doc.is_return:
|
||||
asset_status = frappe.db.get_value("Asset", d.asset, "status")
|
||||
if doc.update_stock:
|
||||
frappe.throw(_("'Update Stock' cannot be checked for fixed asset sale"))
|
||||
elif asset_status in ("Scrapped", "Cancelled", "Capitalized"):
|
||||
frappe.throw(
|
||||
_("Row #{0}: Asset {1} cannot be sold, it is already {2}").format(
|
||||
d.idx, d.asset, asset_status
|
||||
)
|
||||
)
|
||||
elif asset_status == "Sold" and not doc.is_return:
|
||||
frappe.throw(_("Row #{0}: Asset {1} is already sold").format(d.idx, d.asset))
|
||||
elif not doc.return_against:
|
||||
frappe.throw(_("Row #{0}: Return Against is required for returning asset").format(d.idx))
|
||||
else:
|
||||
frappe.throw(
|
||||
_("Row #{0}: You must select an Asset for Item {1}.").format(d.idx, d.item_code),
|
||||
title=_("Missing Asset"),
|
||||
def _validate_fixed_asset_item(self, item) -> None:
|
||||
doc = self.doc
|
||||
if not item.asset:
|
||||
frappe.throw(
|
||||
_("Row #{0}: You must select an Asset for Item {1}.").format(item.idx, item.item_code),
|
||||
title=_("Missing Asset"),
|
||||
)
|
||||
|
||||
if doc.is_return:
|
||||
if not doc.return_against:
|
||||
frappe.throw(_("Row #{0}: Return Against is required for returning asset").format(item.idx))
|
||||
return
|
||||
|
||||
if doc.update_stock:
|
||||
frappe.throw(_("'Update Stock' cannot be checked for fixed asset sale"))
|
||||
|
||||
asset_status = frappe.db.get_value("Asset", item.asset, "status")
|
||||
if asset_status in ("Scrapped", "Cancelled", "Capitalized"):
|
||||
frappe.throw(
|
||||
_("Row #{0}: Asset {1} cannot be sold, it is already {2}").format(
|
||||
item.idx, item.asset, asset_status
|
||||
)
|
||||
)
|
||||
if asset_status == "Sold":
|
||||
frappe.throw(_("Row #{0}: Asset {1} is already sold").format(item.idx, item.asset))
|
||||
|
||||
def set_income_account_for_fixed_assets(self) -> None:
|
||||
for item in self.doc.items:
|
||||
|
||||
@@ -13,36 +13,54 @@ def validate_inter_company_party(
|
||||
if not party:
|
||||
return
|
||||
|
||||
if doctype in ["Sales Invoice", "Sales Order"]:
|
||||
partytype, ref_partytype, internal = "Customer", "Supplier", "is_internal_customer"
|
||||
ref_doc = "Purchase Invoice" if doctype == "Sales Invoice" else "Purchase Order"
|
||||
else:
|
||||
partytype, ref_partytype, internal = "Supplier", "Customer", "is_internal_supplier"
|
||||
ref_doc = "Sales Invoice" if doctype == "Purchase Invoice" else "Sales Order"
|
||||
config = _get_inter_company_party_config(doctype)
|
||||
|
||||
if inter_company_reference:
|
||||
doc = frappe.get_doc(ref_doc, inter_company_reference)
|
||||
ref_party = doc.supplier if doctype in ["Sales Invoice", "Sales Order"] else doc.customer
|
||||
if frappe.db.get_value(partytype, {"represents_company": doc.company}, "name") != party:
|
||||
frappe.throw(_("Invalid {0} for Inter Company Transaction.").format(_(partytype)))
|
||||
if frappe.get_cached_value(ref_partytype, ref_party, "represents_company") != company:
|
||||
frappe.throw(_("Invalid Company for Inter Company Transaction."))
|
||||
_validate_against_reference(config, party, company, inter_company_reference)
|
||||
elif frappe.db.get_value(config.partytype, {"name": party, config.internal: 1}, "name") == party:
|
||||
_validate_internal_party_company(config.partytype, party, company)
|
||||
|
||||
elif frappe.db.get_value(partytype, {"name": party, internal: 1}, "name") == party:
|
||||
companies = [
|
||||
d.company
|
||||
for d in frappe.get_all(
|
||||
"Allowed To Transact With",
|
||||
fields=["company"],
|
||||
filters={"parenttype": partytype, "parent": party},
|
||||
)
|
||||
]
|
||||
if company not in companies:
|
||||
frappe.throw(
|
||||
_(
|
||||
"{0} not allowed to transact with {1}. Please change the Company or add the Company in the 'Allowed To Transact With'-Section in the Customer record."
|
||||
).format(_(partytype), company)
|
||||
)
|
||||
|
||||
def _get_inter_company_party_config(doctype: str) -> "frappe._dict":
|
||||
if doctype in ["Sales Invoice", "Sales Order"]:
|
||||
return frappe._dict(
|
||||
partytype="Customer",
|
||||
ref_partytype="Supplier",
|
||||
internal="is_internal_customer",
|
||||
ref_doc="Purchase Invoice" if doctype == "Sales Invoice" else "Purchase Order",
|
||||
)
|
||||
return frappe._dict(
|
||||
partytype="Supplier",
|
||||
ref_partytype="Customer",
|
||||
internal="is_internal_supplier",
|
||||
ref_doc="Sales Invoice" if doctype == "Purchase Invoice" else "Sales Order",
|
||||
)
|
||||
|
||||
|
||||
def _validate_against_reference(config, party: str, company: str, inter_company_reference: str) -> None:
|
||||
doc = frappe.get_doc(config.ref_doc, inter_company_reference)
|
||||
ref_party = doc.supplier if config.partytype == "Customer" else doc.customer
|
||||
if frappe.db.get_value(config.partytype, {"represents_company": doc.company}, "name") != party:
|
||||
frappe.throw(_("Invalid {0} for Inter Company Transaction.").format(_(config.partytype)))
|
||||
if frappe.get_cached_value(config.ref_partytype, ref_party, "represents_company") != company:
|
||||
frappe.throw(_("Invalid Company for Inter Company Transaction."))
|
||||
|
||||
|
||||
def _validate_internal_party_company(partytype: str, party: str, company: str) -> None:
|
||||
companies = [
|
||||
d.company
|
||||
for d in frappe.get_all(
|
||||
"Allowed To Transact With",
|
||||
fields=["company"],
|
||||
filters={"parenttype": partytype, "parent": party},
|
||||
)
|
||||
]
|
||||
if company not in companies:
|
||||
frappe.throw(
|
||||
_(
|
||||
"{0} not allowed to transact with {1}. Please change the Company or add the Company in the 'Allowed To Transact With'-Section in the Customer record."
|
||||
).format(_(partytype), company)
|
||||
)
|
||||
|
||||
|
||||
def update_linked_doc(doctype: str, name: str, inter_company_reference: str | None) -> None:
|
||||
|
||||
@@ -3667,6 +3667,49 @@ class TestSalesInvoice(ERPNextTestSuite):
|
||||
self.assertEqual(expected_values[i][2], schedule.accumulated_depreciation_amount)
|
||||
self.assertTrue(schedule.journal_entry)
|
||||
|
||||
def test_fixed_asset_sale_validations(self):
|
||||
from erpnext.accounts.doctype.sales_invoice.services.fixed_assets import FixedAssetService
|
||||
|
||||
asset = create_asset(item_code="Macbook Pro", calculate_depreciation=0, submit=1)
|
||||
|
||||
def asset_invoice(asset_name, **kwargs):
|
||||
si = create_sales_invoice(
|
||||
item_code="Macbook Pro", asset=asset_name, qty=1, rate=90000, do_not_save=True, **kwargs
|
||||
)
|
||||
si.items[0].is_fixed_asset = 1
|
||||
return si
|
||||
|
||||
with self.subTest("item without an asset is rejected"):
|
||||
si = asset_invoice(None)
|
||||
self.assertRaises(frappe.ValidationError, FixedAssetService(si).validate_fixed_asset)
|
||||
|
||||
with self.subTest("update stock on an asset sale is rejected"):
|
||||
si = asset_invoice(asset.name, update_stock=1)
|
||||
self.assertRaises(frappe.ValidationError, FixedAssetService(si).validate_fixed_asset)
|
||||
|
||||
with self.subTest("return without return-against is rejected"):
|
||||
si = asset_invoice(asset.name, is_return=1)
|
||||
self.assertRaises(frappe.ValidationError, FixedAssetService(si).validate_fixed_asset)
|
||||
|
||||
for bad_status in ("Sold", "Scrapped", "Cancelled", "Capitalized"):
|
||||
with self.subTest(f"selling a {bad_status} asset is rejected"):
|
||||
frappe.db.set_value("Asset", asset.name, "status", bad_status)
|
||||
si = asset_invoice(asset.name)
|
||||
self.assertRaises(frappe.ValidationError, FixedAssetService(si).validate_fixed_asset)
|
||||
frappe.db.set_value("Asset", asset.name, "status", "Submitted")
|
||||
|
||||
def test_fixed_asset_restore_note_text(self):
|
||||
from erpnext.accounts.doctype.sales_invoice.services.fixed_assets import FixedAssetService
|
||||
|
||||
asset = frappe._dict(doctype="Asset", name="_Test Asset For Note")
|
||||
si = create_sales_invoice(do_not_save=True)
|
||||
|
||||
si.is_return = 1
|
||||
self.assertIn("returned", FixedAssetService(si)._get_note_for_asset_return(asset))
|
||||
|
||||
si.is_return = 0
|
||||
self.assertIn("restored", FixedAssetService(si)._get_note_for_asset_return(asset))
|
||||
|
||||
def test_sales_invoice_against_supplier(self):
|
||||
from erpnext.accounts.doctype.opening_invoice_creation_tool.test_opening_invoice_creation_tool import (
|
||||
make_customer,
|
||||
|
||||
Reference in New Issue
Block a user