mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-14 15:11:52 +00:00
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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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():
|
||||
|
||||
Reference in New Issue
Block a user