nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-09-18 02:55:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: negative stock value for moving average item with mixed batchwise valuation (#59099)

Browse Source 
* fix: negative stock value for moving average item with mixed batchwise valuation

* chore: remove redundant docstring

* test: restore frappe flags in a finally block
This commit is contained in:
rohitwaghchaure
2026-09-16 14:49:01 +05:30
committed by GitHub
parent f09ce0583f
commit 262fdf3e69
2 changed files with 249 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

133
erpnext/stock/deprecated_serial_batch.py
View File

@@ -7,6 +7,7 @@ from frappe.query_builder.functions import Locate, Sum
from frappe.utils import flt, nowtime
from pypika import Order
from pypika.functions import Coalesce, Concat
from pypika.terms import ExistsCriterion
from erpnext.deprecation_dumpster import deprecated
@@ -182,37 +183,79 @@ class DeprecatedBatchNoValuation:
self.set_balance_value_for_non_batchwise_valuation_batches()
fallback_rate = self.get_pooled_fallback_rate()
for batch_no, ledger in self.batch_nos.items():
if batch_no not in self.non_batchwise_valuation_batches:
continue
if not self.non_batchwise_balance_qty:
continue
if not self.non_batchwise_balance_qty.get(batch_no):
self.batch_avg_rate[batch_no] = 0.0
self.stock_value_differece[batch_no] = 0.0
else:
self.batch_avg_rate[batch_no] = (
self.non_batchwise_balance_value[batch_no] / self.non_batchwise_balance_qty[batch_no]
)
self.stock_value_differece[batch_no] = self.non_batchwise_balance_value
self.batch_avg_rate[batch_no] = self.get_non_batchwise_avg_rate(batch_no, fallback_rate)
self.stock_value_differece[batch_no] = flt(self.non_batchwise_balance_value.get(batch_no))
stock_value_change = self.batch_avg_rate[batch_no] * ledger.qty
self.stock_value_change += stock_value_change
self.non_batchwise_balance_value[batch_no] -= stock_value_change
self.non_batchwise_balance_qty[batch_no] -= ledger.qty
# ledger.qty is negative for outward entries, so adding drains the pool
self.non_batchwise_balance_value[batch_no] += stock_value_change
self.non_batchwise_balance_qty[batch_no] += ledger.qty
# on the legacy batch_no field path the ledger is the Stock Ledger Entry itself,
# so there is no Serial and Batch Entry row to write the rate back to
if not self.sle.get("serial_and_batch_bundle") or not ledger.get("name"):
continue
frappe.db.set_value(
"Serial and Batch Entry",
ledger.name,
{
"stock_value_difference": stock_value_change,
"incoming_rate": self.batch_avg_rate[batch_no],
# every reader of incoming_rate takes abs(), keep the stored value in step
"incoming_rate": abs(self.batch_avg_rate[batch_no]),
},
)
def get_non_batchwise_avg_rate(self, batch_no, fallback_rate):
balance_qty = flt(self.non_batchwise_balance_qty.get(batch_no))
balance_value = flt(self.non_batchwise_balance_value.get(batch_no))
# The value and the qty of a batch are summed independently over its whole history
# and the two can drift apart: outward entries posted before batch level valuation
# existed were priced at the pooled warehouse rate, and a Stock Reconciliation posts
# value with no matching qty. A drained or a negative pool would otherwise yield a
# negative or an exploding rate, which is read back as abs() by the callers and
# silently overdraws the warehouse stock value.
if balance_qty > 0 and balance_value > 0:
return balance_value / balance_qty
return fallback_rate
def get_pooled_fallback_rate(self):
"""Moving average rate of the stock that is not valued batch wise.
`last_sle` carries the balance of the whole warehouse, so the batches that are
valued batch wise have to be netted off before it can price the ones that are not.
"""
last_sle = self.last_sle or frappe._dict()
total_qty = flt(last_sle.qty_after_transaction)
total_value = flt(last_sle.stock_value)
qty, value = total_qty, total_value
for batch_no in self.batchwise_valuation_batches:
qty -= flt(self.available_qty.get(batch_no))
value -= flt(self.stock_value_differece.get(batch_no))
if qty > 0 and value > 0:
return value / qty
# The batchwise batches can account for more than the warehouse holds when the
# legacy ledger is itself inconsistent, which is what an overdrawn history leaves
# behind. The plain warehouse rate is then the best basis left.
if total_qty > 0 and total_value > 0:
return total_value / total_qty
return 0.0
@deprecated(
"erpnext.stock.serial_batch_bundle.BatchNoValuation.set_balance_value_for_non_batchwise_valuation_batches",
"unknown",
@@ -285,13 +328,9 @@ class DeprecatedBatchNoValuation:
query = query.where(sle.name != self.sle.name)
# Moving Average items with no Use Batch wise Valuation but want to use batch wise valuation
moving_avg_item_non_batch_value = False
if valuation_method := self.get_valuation_method(self.sle.item_code):
if valuation_method == "Moving Average" and not frappe.db.get_single_value(
"Stock Settings", "do_not_use_batchwise_valuation"
):
query = query.where(batch.use_batchwise_valuation == 0)
moving_avg_item_non_batch_value = True
moving_avg_item_non_batch_value = self.use_batch_pool_for_moving_average()
if moving_avg_item_non_batch_value:
query = query.where(batch.use_batchwise_valuation == 0)
batch_data = query.run(as_dict=True)
for d in batch_data:
@@ -373,11 +412,37 @@ class DeprecatedBatchNoValuation:
if not posting_datetime and self.sle.posting_date:
posting_datetime = get_combine_datetime(self.sle.posting_date, self.sle.posting_time)
sle_creation = self.sle.creation
if not sle_creation and self.sle.get("serial_and_batch_bundle"):
sle_creation = frappe.db.get_value(
"Stock Ledger Entry",
{"serial_and_batch_bundle": self.sle.serial_and_batch_bundle, "is_cancelled": 0},
"creation",
)
if not sle_creation:
# the current entry is not in the ledger yet, so it sorts after everything posted
# at the same instant; nudge the boundary to take them in, the same way
# set_balance_value_from_sl_entries does, otherwise the two halves of one bundle
# are summed as of two different points in time
posting_datetime = posting_datetime + datetime.timedelta(milliseconds=1)
timestamp_condition = bundle.posting_datetime < posting_datetime
if self.sle.creation:
timestamp_condition |= (bundle.posting_datetime == posting_datetime) & (
bundle.creation < self.sle.creation
if sle_creation:
sle_table = frappe.qb.DocType("Stock Ledger Entry")
# bundle creation and SLE creation are different timelines (a bundle can be
# created much before its SLE), so break the tie on the creation of the
# bundle's own SLE, exactly like BatchNoValuation.get_batch_stock_before_date
timestamp_condition |= (bundle.posting_datetime == posting_datetime) & ExistsCriterion(
frappe.qb.from_(sle_table)
.select(sle_table.name)
.where(
(sle_table.serial_and_batch_bundle == bundle.name)
& (sle_table.is_cancelled == 0)
& (sle_table.creation < sle_creation)
)
)
query = (
@@ -411,13 +476,9 @@ class DeprecatedBatchNoValuation:
query = query.where(bundle.voucher_type != "Pick List")
# Moving Average items with no Use Batch wise Valuation but want to use batch wise valuation
moving_avg_item_non_batch_value = False
if valuation_method := self.get_valuation_method(self.sle.item_code):
if valuation_method == "Moving Average" and not frappe.db.get_single_value(
"Stock Settings", "do_not_use_batchwise_valuation"
):
query = query.where(batch.use_batchwise_valuation == 0)
moving_avg_item_non_batch_value = True
moving_avg_item_non_batch_value = self.use_batch_pool_for_moving_average()
if moving_avg_item_non_batch_value:
query = query.where(batch.use_batchwise_valuation == 0)
batch_data = query.run(as_dict=True)
for d in batch_data:
@@ -440,3 +501,13 @@ class DeprecatedBatchNoValuation:
from erpnext.stock.utils import get_valuation_method
return get_valuation_method(item_code, self.sle.company)
def use_batch_pool_for_moving_average(self):
if not hasattr(self, "_use_batch_pool_for_moving_average"):
self._use_batch_pool_for_moving_average = self.get_valuation_method(
self.sle.item_code
) == "Moving Average" and not frappe.get_single_value(
"Stock Settings", "do_not_use_batchwise_valuation"
)
return self._use_batch_pool_for_moving_average

147
erpnext/stock/doctype/serial_and_batch_bundle/test_serial_and_batch_bundle.py
View File

@@ -494,6 +494,153 @@ class TestSerialandBatchBundle(ERPNextTestSuite):
self.assertEqual(flt(sle.stock_value), 0.0)
self.assertEqual(flt(sle.qty_after_transaction), 0.0)
def test_moving_avg_item_with_mixed_batchwise_valuation(self):
"""A Moving Average item holding both batchwise and non batchwise batches.
The non batchwise batches were consumed at the pooled warehouse rate before batch
level valuation existed, so the sum of their stock value differences no longer
tracks the sum of their quantities. Valuing a later outward off that drained pool
used to hand back a negative rate, which the callers read as abs() and used to
overdraw the warehouse, driving stock value negative.
"""
frappe.db.set_single_value("Stock Settings", "do_not_use_batchwise_valuation", 0)
item_code = "Old Batch Item Mixed Valuation 1"
make_item(
item_code,
{
"has_batch_no": 1,
"batch_number_series": "TEST-MIX-BAT-VAL-.#####",
"create_new_batch": 1,
"is_stock_item": 1,
"valuation_method": "Moving Average",
},
)
warehouse = "_Test Warehouse - _TC"
non_batchwise_batch = "TEST-MIX-BAT-VAL-00001"
batchwise_batch = "TEST-MIX-BAT-VAL-00002"
for batch_id, use_batchwise_valuation in (
(non_batchwise_batch, 0),
(batchwise_batch, 1),
):
if not frappe.db.exists("Batch", batch_id):
batch_doc = frappe.get_doc(
{
"doctype": "Batch",
"batch_id": batch_id,
"item": item_code,
"use_batchwise_valuation": use_batchwise_valuation,
}
).insert(ignore_permissions=True)
batch_doc.db_set("use_batchwise_valuation", use_batchwise_valuation)
# ERPNextTestSuite.tearDown only rolls the db back, it does not restore
# frappe.local.flags, so these have to be put back even if a submit raises
previous_flags = (
frappe.flags.ignore_serial_batch_bundle_validation,
frappe.flags.use_serial_and_batch_fields,
)
frappe.flags.ignore_serial_batch_bundle_validation = True
frappe.flags.use_serial_and_batch_fields = True
# Legacy ledger, written the way the pre batch-level-valuation code posted it:
# in 20 @ 50 of the non batchwise batch -> warehouse 20 qty / 1000
# in 20 @ 450 of the batchwise batch -> warehouse 40 qty / 10000, rate 250
# out 20 of the non batchwise batch, priced at the pooled rate of 250
# which leaves the non batchwise batch with a pool of -4000 value against 0 qty.
legacy_entries = [
(non_batchwise_batch, 20, 1000, 20, 1000),
(batchwise_batch, 20, 9000, 40, 10000),
(non_batchwise_batch, -20, -5000, 20, 5000),
]
try:
for batch_id, qty, svd, qty_after_transaction, stock_value in legacy_entries:
doc = frappe.get_doc(
{
"doctype": "Stock Ledger Entry",
"posting_date": today(),
"posting_time": nowtime(),
"batch_no": batch_id,
"incoming_rate": (svd / qty) if qty > 0 else 0,
"qty_after_transaction": qty_after_transaction,
"stock_value_difference": svd,
"stock_value": stock_value,
"balance_value": stock_value,
"valuation_rate": stock_value / qty_after_transaction,
"actual_qty": qty,
"item_code": item_code,
"warehouse": warehouse,
}
)
doc.set_posting_datetime()
doc.flags.ignore_permissions = True
doc.flags.ignore_mandatory = True
doc.flags.ignore_links = True
doc.flags.ignore_validate = True
doc.submit()
finally:
(
frappe.flags.ignore_serial_batch_bundle_validation,
frappe.flags.use_serial_and_batch_fields,
) = previous_flags
# Refill the drained non batchwise batch, then consume it back out.
make_stock_entry(
item_code=item_code,
target=warehouse,
qty=10,
rate=50,
batch_no=non_batchwise_batch,
use_serial_batch_fields=True,
)
se = make_stock_entry(
item_code=item_code,
source=warehouse,
qty=10,
batch_no=non_batchwise_batch,
use_serial_batch_fields=True,
)
sle = frappe.db.get_value(
"Stock Ledger Entry",
{"item_code": item_code, "is_cancelled": 0, "voucher_no": se.name},
["qty_after_transaction", "stock_value", "stock_value_difference"],
as_dict=True,
)
self.assertEqual(flt(sle.qty_after_transaction), 20.0)
# The non batchwise batch is left holding a pool of -3500 against 10 qty, so its
# own rate works out to -350. That used to be taken as abs() = 350 and the 10 units
# drew 3500 out of the warehouse, against the 50 each they were actually bought at.
# The drained pool is rejected now and the warehouse rate of 5500 / 30 is used.
self.assertEqual(flt(sle.stock_value_difference, 2), -1833.33)
self.assertEqual(flt(sle.stock_value, 2), 3666.67)
self.assertGreaterEqual(flt(sle.stock_value), 0.0)
# a batch is never valued at a negative rate, and the rate stored on the ledger
# entry stays in step with the sign every reader applies to it
bundle = frappe.db.get_value(
"Stock Ledger Entry",
{"item_code": item_code, "is_cancelled": 0, "voucher_no": se.name},
"serial_and_batch_bundle",
)
incoming_rate = frappe.db.get_value(
"Serial and Batch Entry",
{"parent": bundle, "batch_no": non_batchwise_batch},
"incoming_rate",
)
self.assertGreaterEqual(flt(incoming_rate), 0.0)
self.assertEqual(flt(incoming_rate, 2), 183.33)
def test_old_serial_no_valuation(self):
from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt