Merge pull request #57487 from frappe/mergify/bp/version-16-hotfix/pr-57485

fix: pool batch slot values on every run, not only when negative (backport #57485)

erpnext/stock/report/stock_ageing/stock_ageing.py

@@ -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

erpnext/stock/report/stock_ageing/test_stock_ageing.py

@@ -5,7 +5,13 @@ from unittest.mock import patch
 
 import frappe
 
-from erpnext.stock.report.stock_ageing.stock_ageing import FIFOSlots, format_report_data, get_average_age
+from erpnext.stock.report.stock_ageing.stock_ageing import (
+	BATCH_SLOT_QTY_INDEX,
+	BATCH_SLOT_VALUE_INDEX,
+	FIFOSlots,
+	format_report_data,
+	get_average_age,
+)
 from erpnext.tests.utils import ERPNextTestSuite
 
 
@@ -565,10 +571,11 @@ class TestStockAgeing(ERPNextTestSuite):
 			],
 		)
 
-	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(
@@ -609,12 +616,112 @@ class TestStockAgeing(ERPNextTestSuite):
 		queue = slots[item_code]["fifo_queue"]
 
 		self.assertEqual(
-			queue,
+			[slot[:4] for slot in 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"],
+				[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", 50.0],
+				[batch_no, 1, 10.0, "2021-12-01", 50.0],
+			],
+		)
+
+	def test_batch_pooling_preserves_total_on_repeating_rate(self):
+		"""Ledger (same wh, batch B): [+3 @ 100/3, +6 @ 0, +2 @ 0]
+		The pooled rate does not terminate, so assert the redistributed
+		slot values still add back to the batch total."""
+		from erpnext.stock.doctype.item.test_item import make_item
+
+		item_code = make_item(
+			"Test Stock Ageing Batch Pool Residual",
+			{"is_stock_item": 1, "has_batch_no": 1, "valuation_method": "FIFO"},
+		).name
+
+		batch_no = "SA-POOL-RESIDUAL-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", 3, 3, 100),
+			make_sle("2021-12-02", "002", 6, 9, 0),
+			make_sle("2021-12-03", "003", 2, 11, 0),
+		]
+
+		slots = FIFOSlots(self.filters, sle).generate()
+		queue = slots[item_code]["fifo_queue"]
+
+		self.assertEqual([slot[BATCH_SLOT_QTY_INDEX] for slot in queue], [3.0, 6.0, 2.0])
+		self.assertEqual(sum(slot[BATCH_SLOT_VALUE_INDEX] for slot in queue), 100.0)
 
 	def test_batch_issue_at_pooled_rate_keeps_slot_values_positive(self):
 		"""Ledger (same wh, batch B): [+10 @ 0, +10 @ 10, -4 @ pooled 5]