From e4f9c664a879717d4bf7160ebf9cf5d6f0a4b44b Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 11 Aug 2026 11:35:24 +0530 Subject: [PATCH] refactor: stock write-path cleanups (SLE/Bin chokepoint groundwork) (#57980) * fix: skip zero-qty rows in make_sl_entries instead of reusing the previous entry A row with zero actual_qty that is not a Stock Reconciliation never gets an SLE, but the loop body still ran with the previous iteration's sle_doc: repost_current_voucher and the bin update executed twice for the previous row, or the whole call crashed with UnboundLocalError when the zero-qty row came first. Skip such rows entirely. * refactor: remove dead update_entries_after.update_bin_data No callers anywhere in the codebase; it duplicates update_bin() with subtly different semantics (no update_modified) and would only invite accidental resurrection as a second Bin write path. * refactor: rename bin.update_qty to update_qty_from_sle Two unrelated functions circulated under the name update_bin_qty: bin.update_qty (recomputes quantities from the ledger, aliased on import in stock_ledger.py) and stock_balance.update_bin_qty (writes caller-supplied absolute values, imported by six modules). Give the SLE-driven one a name that states its semantics and drop the alias. --- erpnext/stock/doctype/bin/bin.py | 8 +++++- .../test_stock_ledger_entry.py | 26 +++++++++++++++++++ erpnext/stock/stock_ledger.py | 21 ++++----------- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/erpnext/stock/doctype/bin/bin.py b/erpnext/stock/doctype/bin/bin.py index b004975d2fe..7b6dab4ae0e 100644 --- a/erpnext/stock/doctype/bin/bin.py +++ b/erpnext/stock/doctype/bin/bin.py @@ -258,7 +258,13 @@ def get_bin_details(bin_name): ) -def update_qty(bin_name, args): +def update_qty_from_sle(bin_name, args): + """Refresh the Bin's quantity fields after an SLE has been processed. + + Distinct from ``stock_balance.update_bin_qty``, which writes caller-supplied + absolute values; this recomputes every quantity from the ledger and open + documents. + """ from erpnext.controllers.stock_controller import future_sle_exists bin_details = get_bin_details(bin_name) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index c89f514a3f4..4171e06118a 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -1573,6 +1573,32 @@ class TestStockLedgerEntry(ERPNextTestSuite, StockTestMixin): item_code=item_code, source=warehouse, qty=470.84, rate=100, posting_date=add_days(today(), -1) ) + def test_zero_qty_row_is_skipped(self): + """A zero-qty non-reconciliation row must be skipped entirely: no SLE, + no crash, no reprocessing of the previous row's entry.""" + from erpnext.stock.stock_ledger import make_sl_entries + + item = make_item(properties={"is_stock_item": 1}) + voucher_no = f"zero-qty-{uuid4()}" + + make_sl_entries( + [ + frappe._dict( + item_code=item.name, + warehouse="_Test Warehouse - _TC", + company="_Test Company", + posting_date=today(), + posting_time="12:00:00", + voucher_type="Stock Entry", + voucher_no=voucher_no, + actual_qty=0, + stock_uom=item.stock_uom, + ) + ] + ) + + self.assertFalse(frappe.db.exists("Stock Ledger Entry", {"voucher_no": voucher_no})) + def create_repack_entry(**args): args = frappe._dict(args) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 4d5db75f6b6..cc192d4dba9 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -26,7 +26,7 @@ from frappe.utils import ( ) import erpnext -from erpnext.stock.doctype.bin.bin import update_qty as update_bin_qty +from erpnext.stock.doctype.bin.bin import update_qty_from_sle from erpnext.stock.doctype.inventory_dimension.inventory_dimension import get_inventory_dimensions from erpnext.stock.doctype.serial_and_batch_bundle.serial_and_batch_bundle import ( get_auto_batch_nos, @@ -172,9 +172,10 @@ def make_sl_entries(sl_entries, allow_negative_stock=False, via_landed_cost_vouc ) sle["outgoing_rate"] = 0.0 - if sle.get("actual_qty") or sle.get("voucher_type") == "Stock Reconciliation": - sle_doc = make_entry(sle, allow_negative_stock, via_landed_cost_voucher) + if not (sle.get("actual_qty") or sle.get("voucher_type") == "Stock Reconciliation"): + continue + sle_doc = make_entry(sle, allow_negative_stock, via_landed_cost_voucher) args = sle_doc.as_dict() args["posting_datetime"] = get_combine_datetime(args.posting_date, args.posting_time) @@ -189,7 +190,7 @@ def make_sl_entries(sl_entries, allow_negative_stock=False, via_landed_cost_vouc repost_current_voucher( args, allow_negative_stock, via_landed_cost_voucher, cancelled=cancelled ) - update_bin_qty(bin_name, args) + update_qty_from_sle(bin_name, args) else: frappe.msgprint( _("Item {0} ignored since it is not a stock item").format(args.get("item_code")) @@ -1936,18 +1937,6 @@ class update_entries_after: else: raise NegativeStockError(message) - def update_bin_data(self, sle): - bin_name = get_or_make_bin(sle.item_code, sle.warehouse) - values_to_update = { - "actual_qty": sle.qty_after_transaction, - "stock_value": sle.stock_value, - } - - if sle.valuation_rate is not None: - values_to_update["valuation_rate"] = sle.valuation_rate - - frappe.db.set_value("Bin", bin_name, values_to_update) - def update_bin(self): # update bin for each warehouse for (item_code, warehouse), data in self.prev_sle_dict.items():