From a1fae959ed9ef6b02779dee8a0ff5b1b7f39d7c7 Mon Sep 17 00:00:00 2001 From: khushi8112 Date: Thu, 9 Jul 2026 14:14:44 +0530 Subject: [PATCH] fix: match depreciation schedule rows at currency precision to avoid duplicate JEs (cherry picked from commit 947ed5dfe19a0082cdcc0a387ddf45834afcc894) # Conflicts: # erpnext/accounts/doctype/journal_entry/services/asset_service.py --- .../journal_entry/services/asset_service.py | 201 ++++++++++++++++++ erpnext/assets/doctype/asset/test_asset.py | 40 ++++ 2 files changed, 241 insertions(+) create mode 100644 erpnext/accounts/doctype/journal_entry/services/asset_service.py diff --git a/erpnext/accounts/doctype/journal_entry/services/asset_service.py b/erpnext/accounts/doctype/journal_entry/services/asset_service.py new file mode 100644 index 00000000000..c0b954233af --- /dev/null +++ b/erpnext/accounts/doctype/journal_entry/services/asset_service.py @@ -0,0 +1,201 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe import _ +from frappe.utils import flt + +from erpnext.assets.doctype.asset_depreciation_schedule.asset_depreciation_schedule import ( + get_depr_schedule, +) + + +class AssetService: + """Keeps Assets in sync with the Journal Entries that depreciate, dispose or + adjust them. + + On submit of a Depreciation Entry it reduces the asset value and links the + depreciation schedule; on submit of an Asset Disposal it marks the asset + disposed. On cancel it reverses those links. It also guards cancellation of + Journal Entries tied to asset scrapping or value adjustments. + """ + + def __init__(self, doc) -> None: + self.doc = doc + + def validate_depr_account_and_depr_entry_voucher_type(self) -> None: + """A depreciation account requires voucher type Depreciation Entry and an Expense account.""" + for d in self.doc.get("accounts"): + if d.account_type == "Depreciation": + if self.doc.voucher_type != "Depreciation Entry": + frappe.throw( + _("Journal Entry type should be set as Depreciation Entry for asset depreciation") + ) + + if frappe.get_cached_value("Account", d.account, "root_type") != "Expense": + frappe.throw(_("Account {0} should be of type Expense").format(d.account)) + + def has_asset_adjustment_entry(self) -> None: + """Block cancellation while a submitted Asset Value Adjustment links to this entry.""" + if self.doc.flags.get("via_asset_value_adjustment"): + return + + asset_value_adjustment = frappe.db.get_value( + "Asset Value Adjustment", {"docstatus": 1, "journal_entry": self.doc.name}, "name" + ) + if asset_value_adjustment: + frappe.throw( + _( + "Cannot cancel this document as it is linked with the submitted Asset Value Adjustment {0}. Please cancel the Asset Value Adjustment to continue." + ).format(frappe.utils.get_link_to_form("Asset Value Adjustment", asset_value_adjustment)) + ) + + def update_asset_value(self) -> None: + """Apply the entry's effect to its linked assets on submit (depreciation or disposal).""" + self.update_asset_on_depreciation() + self.update_asset_on_disposal() + + def update_asset_on_depreciation(self) -> None: + """Reduce each depreciated asset's value and link the depreciation schedule row.""" + if self.doc.voucher_type != "Depreciation Entry": + return + + for d in self.doc.get("accounts"): + if ( + d.reference_type == "Asset" + and d.reference_name + and frappe.get_cached_value("Account", d.account, "root_type") == "Expense" + and d.debit + ): + asset = frappe.get_cached_doc("Asset", d.reference_name) + + if asset.calculate_depreciation: + self.update_journal_entry_link_on_depr_schedule(asset, d) + self.update_value_after_depreciation(asset, d.debit) + + asset.db_set("value_after_depreciation", asset.value_after_depreciation - d.debit) + asset.set_status() + asset.set_total_booked_depreciations() + + def update_value_after_depreciation(self, asset, depr_amount: float) -> None: + """Subtract the depreciation amount from the asset's relevant finance book.""" + fb_idx = 1 + if self.doc.finance_book: + for fb_row in asset.get("finance_books"): + if fb_row.finance_book == self.doc.finance_book: + fb_idx = fb_row.idx + break + fb_row = asset.get("finance_books")[fb_idx - 1] + fb_row.value_after_depreciation -= depr_amount + frappe.db.set_value( + "Asset Finance Book", fb_row.name, "value_after_depreciation", fb_row.value_after_depreciation + ) + + def update_journal_entry_link_on_depr_schedule(self, asset, je_row) -> None: + """Stamp this entry onto the matching (date + amount) depreciation schedule row.""" + depr_schedule = get_depr_schedule(asset.name, "Active", self.doc.finance_book) + precision = je_row.precision("debit") + for d in depr_schedule or []: + if ( + d.schedule_date == self.doc.posting_date + and not d.journal_entry + and flt(d.depreciation_amount, precision) == flt(je_row.debit, precision) + ): + frappe.db.set_value("Depreciation Schedule", d.name, "journal_entry", self.doc.name) + + def update_asset_on_disposal(self) -> None: + """Mark each referenced asset disposed (date + scrap entry) on an Asset Disposal.""" + if self.doc.voucher_type == "Asset Disposal": + disposed_assets = [] + for d in self.doc.get("accounts"): + if ( + d.reference_type == "Asset" + and d.reference_name + and d.reference_name not in disposed_assets + ): + frappe.db.set_value( + "Asset", + d.reference_name, + { + "disposal_date": self.doc.posting_date, + "journal_entry_for_scrap": self.doc.name, + }, + ) + asset_doc = frappe.get_doc("Asset", d.reference_name) + asset_doc.set_status() + disposed_assets.append(d.reference_name) + + def unlink_asset_reference(self) -> None: + """On cancel, reverse depreciation links and block cancelling an asset-scrap entry.""" + for d in self.doc.get("accounts"): + if self._is_depreciation_asset_row(d): + self._reverse_asset_depreciation(d) + elif ( + self.doc.voucher_type == "Journal Entry" and d.reference_type == "Asset" and d.reference_name + ): + self._block_scrap_journal_cancel(d) + + def _is_depreciation_asset_row(self, d) -> bool: + return bool( + self.doc.voucher_type == "Depreciation Entry" + and d.reference_type == "Asset" + and d.reference_name + and frappe.get_cached_value("Account", d.account, "root_type") == "Expense" + and d.debit + ) + + def _reverse_asset_depreciation(self, d) -> None: + """Add the depreciation amount back to the asset and unlink its schedule row.""" + asset = frappe.get_doc("Asset", d.reference_name) + + if asset.calculate_depreciation and not self._restore_scheduled_depreciation(asset, d.debit): + self._restore_finance_book_value(asset, d.debit) + + asset.db_set("value_after_depreciation", asset.value_after_depreciation + d.debit) + asset.set_status() + asset.set_total_booked_depreciations() + + def _restore_scheduled_depreciation(self, asset, debit: float) -> bool: + """Unlink this entry from the depreciation schedule and credit back its finance book. + + Returns True if a matching scheduled depreciation was found. + """ + for fb_row in asset.get("finance_books"): + depr_schedule = get_depr_schedule(asset.name, "Active", fb_row.finance_book) + for s in depr_schedule or []: + if s.journal_entry == self.doc.name: + s.db_set("journal_entry", None) + fb_row.value_after_depreciation += debit + fb_row.db_update() + return True + return False + + def _restore_finance_book_value(self, asset, debit: float) -> None: + """Credit the depreciation amount back to the relevant finance book when no schedule matched.""" + fb_idx = 1 + if self.doc.finance_book: + for fb_row in asset.get("finance_books"): + if fb_row.finance_book == self.doc.finance_book: + fb_idx = fb_row.idx + break + + fb_row = asset.get("finance_books")[fb_idx - 1] + fb_row.value_after_depreciation += debit + fb_row.db_update() + + def _block_scrap_journal_cancel(self, d) -> None: + """Prevent cancelling a plain Journal Entry that is an asset's scrap voucher.""" + journal_entry_for_scrap = frappe.db.get_value("Asset", d.reference_name, "journal_entry_for_scrap") + if journal_entry_for_scrap == self.doc.name: + frappe.throw( + _("Journal Entry for Asset scrapping cannot be cancelled. Please restore the Asset.") + ) + + def unlink_asset_adjustment_entry(self) -> None: + """Detach this entry from any Asset Value Adjustment that referenced it.""" + AssetValueAdjustment = frappe.qb.DocType("Asset Value Adjustment") + ( + frappe.qb.update(AssetValueAdjustment) + .set(AssetValueAdjustment.journal_entry, None) + .where(AssetValueAdjustment.journal_entry == self.doc.name) + ).run() diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index 424bf9bac87..b2daddc85df 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -1391,6 +1391,46 @@ class TestDepreciationBasics(AssetSetup): self.assertFalse(depr_schedule[1].journal_entry) self.assertFalse(depr_schedule[2].journal_entry) + def test_depr_schedule_link_matches_at_currency_precision(self): + """A Depreciation Schedule row whose amount carries more decimals than the + company currency (e.g. 25701.202 vs a JE debit of 25701.20) must still be + matched and stamped with the Journal Entry. Comparing at exact float + equality left the link NULL, so the scheduler treated the row as unposted + and created a duplicate Journal Entry on every run. Regression test for + AssetService.update_journal_entry_link_on_depr_schedule().""" + from unittest.mock import MagicMock, patch + + from erpnext.accounts.doctype.journal_entry.services import asset_service as asset_service_module + from erpnext.accounts.doctype.journal_entry.services.asset_service import AssetService + + posting_date = getdate("2021-06-01") + je = frappe._dict(name="JE-DEPR-TEST", finance_book=None, posting_date=posting_date) + service = AssetService(je) + + # JE debit is stored at company currency precision (2 dp)... + je_row = MagicMock() + je_row.debit = 25701.20 + je_row.precision.return_value = 2 + + # ...while the schedule row amount carries a third decimal. + schedule_row = frappe._dict( + name="DS-ROW-1", + schedule_date=posting_date, + journal_entry=None, + depreciation_amount=25701.202, + ) + asset = frappe._dict(name="ASSET-TEST") + + with ( + patch.object(asset_service_module, "get_depr_schedule", return_value=[schedule_row]), + patch.object(frappe.db, "set_value") as mock_set_value, + ): + service.update_journal_entry_link_on_depr_schedule(asset, je_row) + + mock_set_value.assert_called_once_with( + "Depreciation Schedule", "DS-ROW-1", "journal_entry", "JE-DEPR-TEST" + ) + def test_depr_entry_posting_when_depr_expense_account_is_an_expense_account(self): """Tests if the Depreciation Expense Account gets debited and the Accumulated Depreciation Account gets credited when the former's an Expense Account."""