nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-09-24 13:57:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

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

* test: cover fallback rate and equal timestamp valuation paths
This commit is contained in:
rohitwaghchaure
2026-09-16 16:29:06 +05:30
committed by GitHub
parent db2e9efab4
commit a37725b6b9
2 changed files with 427 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

@@ -8,6 +8,7 @@ from frappe.utils import flt, nowtime
from frappe.utils.deprecations import deprecated
from pypika import Order
from pypika.functions import Coalesce
from pypika.terms import ExistsCriterion
class DeprecatedSerialNoValuation:
@@ -149,37 +150,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
def set_balance_value_for_non_batchwise_valuation_batches(self):
if hasattr(self, "prev_sle"):
@@ -242,13 +285,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:
@@ -325,11 +364,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")
# a bundle can be created long before the SLE that consumes it, so the two
# creation timestamps are different timelines; break the tie on the creation
# of the bundle's own SLE instead
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 = (
@@ -363,13 +428,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:
@@ -392,3 +453,13 @@ class DeprecatedBatchNoValuation:
from erpnext.stock.utils import get_valuation_method
return get_valuation_method(item_code)
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.db.get_single_value(
"Stock Settings", "do_not_use_batchwise_valuation"
)
return self._use_batch_pool_for_moving_average

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

@@ -493,6 +493,331 @@ class TestSerialandBatchBundle(FrappeTestCase):
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)
# put the flags back even if a submit raises, so they cannot leak into later tests
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 _make_legacy_sle(self, item_code, warehouse, batch_no, qty, svd, qty_after, stock_value):
doc = frappe.get_doc(
{
"doctype": "Stock Ledger Entry",
"posting_date": add_days(today(), -5),
"posting_time": nowtime(),
"batch_no": batch_no,
"incoming_rate": (svd / qty) if qty > 0 else 0,
"qty_after_transaction": qty_after,
"stock_value_difference": svd,
"stock_value": stock_value,
"balance_value": stock_value,
"valuation_rate": stock_value / qty_after,
"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()
def _make_mixed_valuation_item(self, item_code, series, batches):
frappe.db.set_single_value("Stock Settings", "do_not_use_batchwise_valuation", 0)
make_item(
item_code,
{
"has_batch_no": 1,
"batch_number_series": series,
"create_new_batch": 1,
"is_stock_item": 1,
"valuation_method": "Moving Average",
},
)
for batch_id, use_batchwise_valuation in batches:
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)
def test_moving_avg_non_batchwise_batch_without_history(self):
"""A non batchwise batch that no balance query can find any history for.
The pool comes back empty, so the batch used to be valued at a rate of zero: the
qty would leave the warehouse while none of the value did, quietly inflating the
valuation rate of everything left behind. It falls back to the pooled rate now.
Driven through BatchNoValuation directly because SerialandBatchBundle.validate
rejects an outward for a batch with no stock before valuation is ever reached, so
this branch is only live for reposts and legacy batch_no field entries.
"""
from erpnext.stock.serial_batch_bundle import BatchNoValuation
from erpnext.stock.utils import get_combine_datetime
item_code = "Old Batch Item Mixed Valuation 2"
warehouse = "_Test Warehouse - _TC"
non_batchwise_batch = "TEST-MIX2-BAT-VAL-00001"
batchwise_batch = "TEST-MIX2-BAT-VAL-00002"
self._make_mixed_valuation_item(
item_code,
"TEST-MIX2-BAT-VAL-.#####",
[(non_batchwise_batch, 0), (batchwise_batch, 1)],
)
# only the batchwise batch ever receives stock, so the non batchwise one has no
# history at all for the balance queries to pick up
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
try:
self._make_legacy_sle(item_code, warehouse, batchwise_batch, 20, 4000, 20, 4000)
finally:
(
frappe.flags.ignore_serial_batch_bundle_validation,
frappe.flags.use_serial_and_batch_fields,
) = previous_flags
sle = frappe._dict(
{
"item_code": item_code,
"warehouse": warehouse,
"company": "_Test Company",
"actual_qty": -5,
"posting_date": today(),
"posting_time": nowtime(),
"batch_nos": {non_batchwise_batch: frappe._dict({"qty": -5})},
}
)
sle.posting_datetime = get_combine_datetime(sle.posting_date, sle.posting_time)
sn_obj = BatchNoValuation(sle=sle, item_code=item_code, warehouse=warehouse)
# the warehouse holds 20 @ 200, so the 5 units have to take value out with them
self.assertEqual(flt(sn_obj.batch_avg_rate[non_batchwise_batch], 2), 200.0)
self.assertEqual(flt(sn_obj.stock_value_change, 2), -1000.0)
def test_moving_avg_non_batchwise_batch_equal_timestamp_bundles(self):
"""Pins the valuation of two bundles posted at the very same instant.
The bundle balance query and the stock ledger balance query disagreed on whether an
entry sharing the outward's timestamp came before it, so one pool could have its qty
and its value summed as of two different points in time. Telling the two apart needs
a bundle whose creation and whose SLE's creation straddle the outward, which does
not arise from ordinary vouchers, so this pins the equal-timestamp result rather
than reproducing that divergence.
"""
item_code = "Old Batch Item Mixed Valuation 3"
warehouse = "_Test Warehouse - _TC"
non_batchwise_batch = "TEST-MIX3-BAT-VAL-00001"
self._make_mixed_valuation_item(item_code, "TEST-MIX3-BAT-VAL-.#####", [(non_batchwise_batch, 0)])
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
try:
self._make_legacy_sle(item_code, warehouse, non_batchwise_batch, 10, 1000, 10, 1000)
finally:
(
frappe.flags.ignore_serial_batch_bundle_validation,
frappe.flags.use_serial_and_batch_fields,
) = previous_flags
posting_date, posting_time = today(), "10:00:00"
make_stock_entry(
item_code=item_code,
target=warehouse,
qty=10,
rate=300,
batch_no=non_batchwise_batch,
posting_date=posting_date,
posting_time=posting_time,
use_serial_batch_fields=True,
)
se = make_stock_entry(
item_code=item_code,
source=warehouse,
qty=10,
batch_no=non_batchwise_batch,
posting_date=posting_date,
posting_time=posting_time,
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,
)
# the inward sharing the timestamp counts, so the pool is (1000 + 3000) / 20 = 200
self.assertEqual(flt(sle.qty_after_transaction), 10.0)
self.assertEqual(flt(sle.stock_value_difference, 2), -2000.0)
def test_old_serial_no_valuation(self):
from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt