mirror of
https://github.com/frappe/erpnext.git
synced 2026-07-29 07:28:17 +00:00
fix: pool batch slot values on every run, not only when negative
A batch is one valuation pool, so any per-slot value difference within a
batch is stale detail from the report's own age slots, not real valuation.
The rebalance only ran when consumption had already driven a slot negative,
so a batch whose receipts landed at different rates kept a skewed split
across age buckets (one bucket free, another double-priced) while the total
stayed correct.
Drop the negative-slot precondition and always spread a batch's pooled value
over its slots in proportion to qty. Redistribution preserves group totals,
so buckets still sum to Stock Balance; only the split across ages changes.
(cherry picked from commit cedaaa3a00)
This commit is contained in:
@@ -325,7 +325,7 @@ class FIFOSlots:
|
||||
del stock_ledger_entries
|
||||
|
||||
self._recompute_moving_average_slots()
|
||||
self._rebalance_negative_batch_slots()
|
||||
self._rebalance_batch_slots()
|
||||
|
||||
if not self.filters.get("show_warehouse_wise_stock"):
|
||||
# (Item 1, WH 1), (Item 1, WH 2) => (Item 1)
|
||||
@@ -347,14 +347,14 @@ class FIFOSlots:
|
||||
if is_qty_slot(slot):
|
||||
slot[FIFO_VALUE_INDEX] = flt(slot[FIFO_QTY_INDEX] * rate)
|
||||
|
||||
def _rebalance_negative_batch_slots(self) -> None:
|
||||
def _rebalance_batch_slots(self) -> None:
|
||||
for item_dict in self.item_details.values():
|
||||
if item_dict.get("has_batch_no"):
|
||||
self._rebalance_negative_batch_slot_values(item_dict["fifo_queue"])
|
||||
self._rebalance_batch_slot_values(item_dict["fifo_queue"])
|
||||
|
||||
def _rebalance_negative_batch_slot_values(self, fifo_queue: list) -> None:
|
||||
"""A batch is one valuation pool, so a slot driven negative by consumption
|
||||
at the pooled rate is stale detail: spread the pool value over its slots."""
|
||||
def _rebalance_batch_slot_values(self, fifo_queue: list) -> None:
|
||||
"""A batch is one valuation pool, so per-slot value differences are stale
|
||||
detail: spread the pool value over its slots in proportion to qty."""
|
||||
groups = {}
|
||||
for slot in fifo_queue:
|
||||
if is_batch_slot(slot):
|
||||
@@ -362,12 +362,8 @@ class FIFOSlots:
|
||||
groups.setdefault(key, []).append(slot)
|
||||
|
||||
for slots in groups.values():
|
||||
has_negative_slot = any(
|
||||
flt(slot[BATCH_SLOT_VALUE_INDEX]) < 0 and flt(slot[BATCH_SLOT_QTY_INDEX]) > 0
|
||||
for slot in slots
|
||||
)
|
||||
total_qty = sum(flt(slot[BATCH_SLOT_QTY_INDEX]) for slot in slots)
|
||||
if not has_negative_slot or total_qty <= 0:
|
||||
if total_qty <= 0:
|
||||
continue
|
||||
|
||||
rate = sum(flt(slot[BATCH_SLOT_VALUE_INDEX]) for slot in slots) / total_qty
|
||||
|
||||
@@ -569,10 +569,11 @@ class TestStockAgeing(FrappeTestCase):
|
||||
],
|
||||
)
|
||||
|
||||
def test_partial_batch_reco_keeps_existing_slot_values(self):
|
||||
def test_partial_batch_reco_pools_slot_values(self):
|
||||
"""Ledger (same wh, batch B): [+10 @ 100, single-SLE reco >> 12]
|
||||
The reco entry qty (delta 2) does not cover the whole batch, so
|
||||
stock_value_difference / qty is not the batch rate: skip the rescale."""
|
||||
stock_value_difference / qty is not the batch rate: skip the rescale.
|
||||
The batch total (1400) is untouched, then pooled across both slots."""
|
||||
from erpnext.stock.doctype.item.test_item import make_item
|
||||
|
||||
item_code = make_item(
|
||||
@@ -612,11 +613,64 @@ class TestStockAgeing(FrappeTestCase):
|
||||
slots = FIFOSlots(self.filters, sle).generate()
|
||||
queue = slots[item_code]["fifo_queue"]
|
||||
|
||||
self.assertEqual(
|
||||
[slot[:4] for slot in queue],
|
||||
[
|
||||
[batch_no, 1, 10.0, "2021-12-01"],
|
||||
[batch_no, 1, 2.0, "2021-12-01"],
|
||||
],
|
||||
)
|
||||
self.assertAlmostEqual(queue[0][4], 1166.67, places=2)
|
||||
self.assertAlmostEqual(queue[1][4], 233.33, places=2)
|
||||
|
||||
def test_batch_receipts_at_differing_rates_pool_slot_values(self):
|
||||
"""Ledger (same wh, batch B): [+10 @ 0, +10 @ 10] and no issue.
|
||||
Nothing goes negative, but the batch is one valuation pool, so both
|
||||
age slots carry the pooled rate instead of their receipt value."""
|
||||
from erpnext.stock.doctype.item.test_item import make_item
|
||||
|
||||
item_code = make_item(
|
||||
"Test Stock Ageing Batch Pool Split",
|
||||
{"is_stock_item": 1, "has_batch_no": 1, "valuation_method": "FIFO"},
|
||||
).name
|
||||
|
||||
batch_no = "SA-POOL-SPLIT-BATCH"
|
||||
if not frappe.db.exists("Batch", batch_no):
|
||||
frappe.get_doc({"doctype": "Batch", "batch_id": batch_no, "item": item_code}).insert(
|
||||
ignore_permissions=True
|
||||
)
|
||||
frappe.db.set_value("Batch", batch_no, "use_batchwise_valuation", 1)
|
||||
|
||||
def make_sle(posting_date, voucher_no, actual_qty, qty_after, stock_value_difference):
|
||||
return frappe._dict(
|
||||
name=item_code,
|
||||
actual_qty=actual_qty,
|
||||
qty_after_transaction=qty_after,
|
||||
stock_value_difference=stock_value_difference,
|
||||
valuation_rate=abs(stock_value_difference / actual_qty) if actual_qty else 0,
|
||||
warehouse="WH 1",
|
||||
posting_date=posting_date,
|
||||
voucher_type="Stock Entry",
|
||||
voucher_no=voucher_no,
|
||||
has_serial_no=False,
|
||||
has_batch_no=True,
|
||||
serial_no=None,
|
||||
batch_no=batch_no,
|
||||
)
|
||||
|
||||
sle = [
|
||||
make_sle("2021-12-01", "001", 10, 10, 0),
|
||||
make_sle("2021-12-02", "002", 10, 20, 100),
|
||||
]
|
||||
|
||||
slots = FIFOSlots(self.filters, sle).generate()
|
||||
queue = slots[item_code]["fifo_queue"]
|
||||
|
||||
self.assertEqual(
|
||||
queue,
|
||||
[
|
||||
[batch_no, 1, 10.0, "2021-12-01", 1000.0],
|
||||
[batch_no, 1, 2.0, "2021-12-01", 400.0],
|
||||
[batch_no, 1, 10.0, "2021-12-01", 50.0],
|
||||
[batch_no, 1, 10.0, "2021-12-01", 50.0],
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user