diff --git a/erpnext/accounts/doctype/sales_invoice/services/fixed_assets.py b/erpnext/accounts/doctype/sales_invoice/services/fixed_assets.py index a2843bef7a0..99dfa6fffb9 100644 --- a/erpnext/accounts/doctype/sales_invoice/services/fixed_assets.py +++ b/erpnext/accounts/doctype/sales_invoice/services/fixed_assets.py @@ -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: diff --git a/erpnext/accounts/doctype/sales_invoice/services/inter_company.py b/erpnext/accounts/doctype/sales_invoice/services/inter_company.py index c6e3abaa24b..7d1266ebd68 100644 --- a/erpnext/accounts/doctype/sales_invoice/services/inter_company.py +++ b/erpnext/accounts/doctype/sales_invoice/services/inter_company.py @@ -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: diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 713bf278c66..9deb14eea1d 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -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,