From 12c01e2975094680bad38e74e1432bfcfce2a628 Mon Sep 17 00:00:00 2001
From: Ankush Menat <ankush@frappe.io>
Date: Mon, 4 Apr 2022 15:22:15 +0530
Subject: [PATCH] fix: maintain FIFO queue even if outgoing_rate is not found
 (#30560)

---
 erpnext/stock/tests/test_valuation.py | 35 +++++++++++++++++++++++----
 erpnext/stock/valuation.py            | 18 +++-----------
 2 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/erpnext/stock/tests/test_valuation.py b/erpnext/stock/tests/test_valuation.py
index 506a666c28..e60c1caac3 100644
--- a/erpnext/stock/tests/test_valuation.py
+++ b/erpnext/stock/tests/test_valuation.py
@@ -60,9 +60,9 @@ class TestFIFOValuation(unittest.TestCase):
 		self.queue.remove_stock(1, 5)
 		self.assertEqual(self.queue, [[-1, 5]])
 
-		# XXX
-		self.queue.remove_stock(1, 10)
+		self.queue.remove_stock(1)
 		self.assertTotalQty(-2)
+		self.assertEqual(self.queue, [[-2, 5]])
 
 		self.queue.add_stock(2, 10)
 		self.assertTotalQty(0)
@@ -93,7 +93,7 @@ class TestFIFOValuation(unittest.TestCase):
 		self.queue.remove_stock(3, 20)
 		self.assertEqual(self.queue, [[1, 10], [5, 20]])
 
-	def test_collapsing_of_queue(self):
+	def test_queue_with_unknown_rate(self):
 		self.queue.add_stock(1, 1)
 		self.queue.add_stock(1, 2)
 		self.queue.add_stock(1, 3)
@@ -102,8 +102,7 @@ class TestFIFOValuation(unittest.TestCase):
 		self.assertTotalValue(10)
 
 		self.queue.remove_stock(3, 1)
-		# XXX
-		self.assertEqual(self.queue, [[1, 7]])
+		self.assertEqual(self.queue, [[1, 4]])
 
 	def test_rounding_off(self):
 		self.queue.add_stock(1.0, 1.0)
@@ -172,6 +171,32 @@ class TestFIFOValuation(unittest.TestCase):
 			self.assertTotalQty(total_qty)
 			self.assertTotalValue(total_value)
 
+	@given(stock_queue_generator, st.floats(min_value=0.1, max_value=1e6))
+	def test_fifo_qty_value_nonneg_hypothesis_with_outgoing_rate(self, stock_queue, outgoing_rate):
+		self.queue = FIFOValuation([])
+		total_qty = 0.0
+		total_value = 0.0
+
+		for qty, rate in stock_queue:
+			# don't allow negative stock
+			if qty == 0 or total_qty + qty < 0 or abs(qty) < 0.1:
+				continue
+			if qty > 0:
+				self.queue.add_stock(qty, rate)
+				total_qty += qty
+				total_value += qty * rate
+			else:
+				qty = abs(qty)
+				consumed = self.queue.remove_stock(qty, outgoing_rate)
+				self.assertAlmostEqual(
+					qty, sum(q for q, _ in consumed), msg=f"incorrect consumption {consumed}"
+				)
+				total_qty -= qty
+				total_value -= sum(q * r for q, r in consumed)
+			self.assertTotalQty(total_qty)
+			self.assertTotalValue(total_value)
+			self.assertGreaterEqual(total_value, 0)
+
 
 class TestLIFOValuation(unittest.TestCase):
 	def setUp(self):
diff --git a/erpnext/stock/valuation.py b/erpnext/stock/valuation.py
index 648b218287..35f4f12235 100644
--- a/erpnext/stock/valuation.py
+++ b/erpnext/stock/valuation.py
@@ -60,9 +60,7 @@ class FIFOValuation(BinWiseValuation):
 
 	# specifying the attributes to save resources
 	# ref: https://docs.python.org/3/reference/datamodel.html#slots
-	__slots__ = [
-		"queue",
-	]
+	__slots__ = ["queue"]
 
 	def __init__(self, state: Optional[List[StockBin]]):
 		self.queue: List[StockBin] = state if state is not None else []
@@ -123,15 +121,9 @@ class FIFOValuation(BinWiseValuation):
 						index = idx
 						break
 
-				# If no entry found with outgoing rate, collapse queue
+				# If no entry found with outgoing rate, consume as per FIFO
 				if index is None:  # nosemgrep
-					new_stock_value = sum(d[QTY] * d[RATE] for d in self.queue) - qty * outgoing_rate
-					new_stock_qty = sum(d[QTY] for d in self.queue) - qty
-					self.queue = [
-						[new_stock_qty, new_stock_value / new_stock_qty if new_stock_qty > 0 else outgoing_rate]
-					]
-					consumed_bins.append([qty, outgoing_rate])
-					break
+					index = 0
 			else:
 				index = 0
 
@@ -172,9 +164,7 @@ class LIFOValuation(BinWiseValuation):
 
 	# specifying the attributes to save resources
 	# ref: https://docs.python.org/3/reference/datamodel.html#slots
-	__slots__ = [
-		"stack",
-	]
+	__slots__ = ["stack"]
 
 	def __init__(self, state: Optional[List[StockBin]]):
 		self.stack: List[StockBin] = state if state is not None else []