Merge pull request #29795 from frappe/mergify/bp/version-13-hotfix/pr-29788

fix: Generate Warehouse wise FIFO Queue always and later aggregate if required (backport #29788)

erpnext/stock/report/stock_ageing/stock_ageing.py

@@ -252,6 +252,7 @@ class FIFOSlots:
 			key, fifo_queue, transferred_item_key = self.__init_key_stores(d)
 
 			if d.voucher_type == "Stock Reconciliation":
+				# get difference in qty shift as actual qty
 				prev_balance_qty = self.item_details[key].get("qty_after_transaction", 0)
 				d.actual_qty = flt(d.qty_after_transaction) - flt(prev_balance_qty)
 
@@ -264,12 +265,16 @@ class FIFOSlots:
 
 			self.__update_balances(d, key)
 
+		if not self.filters.get("show_warehouse_wise_stock"):
+			# (Item 1, WH 1), (Item 1, WH 2) => (Item 1)
+			self.item_details = self.__aggregate_details_by_item(self.item_details)
+
 		return self.item_details
 
 	def __init_key_stores(self, row: Dict) -> Tuple:
 		"Initialise keys and FIFO Queue."
 
-		key = (row.name, row.warehouse) if self.filters.get('show_warehouse_wise_stock') else row.name
+		key = (row.name, row.warehouse)
 		self.item_details.setdefault(key, {"details": row, "fifo_queue": []})
 		fifo_queue = self.item_details[key]["fifo_queue"]
 
@@ -338,6 +343,27 @@ class FIFOSlots:
 
 		self.item_details[key]["has_serial_no"] = row.has_serial_no
 
+	def __aggregate_details_by_item(self, wh_wise_data: Dict) -> Dict:
+		"Aggregate Item-Wh wise data into single Item entry."
+		item_aggregated_data = {}
+		for key,row in wh_wise_data.items():
+			item = key[0]
+			if not item_aggregated_data.get(item):
+				item_aggregated_data.setdefault(item, {
+					"details": frappe._dict(),
+					"fifo_queue": [],
+					"qty_after_transaction": 0.0,
+					"total_qty": 0.0
+				})
+			item_row = item_aggregated_data.get(item)
+			item_row["details"].update(row["details"])
+			item_row["fifo_queue"].extend(row["fifo_queue"])
+			item_row["qty_after_transaction"] += flt(row["qty_after_transaction"])
+			item_row["total_qty"] += flt(row["total_qty"])
+			item_row["has_serial_no"] = row["has_serial_no"]
+
+		return item_aggregated_data
+
 	def __get_stock_ledger_entries(self) -> List[Dict]:
 		sle = frappe.qb.DocType("Stock Ledger Entry")
 		item = self.__get_item_query() # used as derived table in sle query

erpnext/stock/report/stock_ageing/stock_ageing_fifo_logic.md

@@ -15,6 +15,7 @@ Here, the balance qty is 70.
 50 qty is (today-the 1st) days old
 20 qty is (today-the 2nd) days old
 
+> Note: We generate FIFO slots warehouse wise as stock reconciliations from different warehouses can cause incorrect values.
 ### Calculation of FIFO Slots
 
 #### Case 1: Outward from sufficient balance qty

erpnext/stock/report/stock_ageing/test_stock_ageing.py

@@ -15,11 +15,12 @@ class TestStockAgeing(ERPNextTestCase):
 		)
 
 	def test_normal_inward_outward_queue(self):
-		"Reference: Case 1 in stock_ageing_fifo_logic.md"
+		"Reference: Case 1 in stock_ageing_fifo_logic.md (same wh)"
 		sle = [
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=30, qty_after_transaction=30,
+				warehouse="WH 1",
 				posting_date="2021-12-01", voucher_type="Stock Entry",
 				voucher_no="001",
 				has_serial_no=False, serial_no=None
@@ -27,6 +28,7 @@ class TestStockAgeing(ERPNextTestCase):
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=20, qty_after_transaction=50,
+				warehouse="WH 1",
 				posting_date="2021-12-02", voucher_type="Stock Entry",
 				voucher_no="002",
 				has_serial_no=False, serial_no=None
@@ -34,6 +36,7 @@ class TestStockAgeing(ERPNextTestCase):
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=(-10), qty_after_transaction=40,
+				warehouse="WH 1",
 				posting_date="2021-12-03", voucher_type="Stock Entry",
 				voucher_no="003",
 				has_serial_no=False, serial_no=None
@@ -50,11 +53,12 @@ class TestStockAgeing(ERPNextTestCase):
 		self.assertEqual(queue[0][0], 20.0)
 
 	def test_insufficient_balance(self):
-		"Reference: Case 3 in stock_ageing_fifo_logic.md"
+		"Reference: Case 3 in stock_ageing_fifo_logic.md (same wh)"
 		sle = [
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=(-30), qty_after_transaction=(-30),
+				warehouse="WH 1",
 				posting_date="2021-12-01", voucher_type="Stock Entry",
 				voucher_no="001",
 				has_serial_no=False, serial_no=None
@@ -62,6 +66,7 @@ class TestStockAgeing(ERPNextTestCase):
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=20, qty_after_transaction=(-10),
+				warehouse="WH 1",
 				posting_date="2021-12-02", voucher_type="Stock Entry",
 				voucher_no="002",
 				has_serial_no=False, serial_no=None
@@ -69,6 +74,7 @@ class TestStockAgeing(ERPNextTestCase):
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=20, qty_after_transaction=10,
+				warehouse="WH 1",
 				posting_date="2021-12-03", voucher_type="Stock Entry",
 				voucher_no="003",
 				has_serial_no=False, serial_no=None
@@ -76,6 +82,7 @@ class TestStockAgeing(ERPNextTestCase):
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=10, qty_after_transaction=20,
+				warehouse="WH 1",
 				posting_date="2021-12-03", voucher_type="Stock Entry",
 				voucher_no="004",
 				has_serial_no=False, serial_no=None
@@ -91,11 +98,16 @@ class TestStockAgeing(ERPNextTestCase):
 		self.assertEqual(queue[0][0], 10.0)
 		self.assertEqual(queue[1][0], 10.0)
 
-	def test_stock_reconciliation(self):
+	def test_basic_stock_reconciliation(self):
+		"""
+		Ledger (same wh): [+30, reco reset >> 50, -10]
+		Bal: 40
+		"""
 		sle = [
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=30, qty_after_transaction=30,
+				warehouse="WH 1",
 				posting_date="2021-12-01", voucher_type="Stock Entry",
 				voucher_no="001",
 				has_serial_no=False, serial_no=None
@@ -103,6 +115,7 @@ class TestStockAgeing(ERPNextTestCase):
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=0, qty_after_transaction=50,
+				warehouse="WH 1",
 				posting_date="2021-12-02", voucher_type="Stock Reconciliation",
 				voucher_no="002",
 				has_serial_no=False, serial_no=None
@@ -110,6 +123,7 @@ class TestStockAgeing(ERPNextTestCase):
 			frappe._dict(
 				name="Flask Item",
 				actual_qty=(-10), qty_after_transaction=40,
+				warehouse="WH 1",
 				posting_date="2021-12-03", voucher_type="Stock Entry",
 				voucher_no="003",
 				has_serial_no=False, serial_no=None
@@ -122,5 +136,112 @@ class TestStockAgeing(ERPNextTestCase):
 		queue = result["fifo_queue"]
 
 		self.assertEqual(result["qty_after_transaction"], result["total_qty"])
+		self.assertEqual(result["total_qty"], 40.0)
 		self.assertEqual(queue[0][0], 20.0)
 		self.assertEqual(queue[1][0], 20.0)
+
+	def test_sequential_stock_reco_same_warehouse(self):
+		"""
+		Test back to back stock recos (same warehouse).
+		Ledger: [reco opening >> +1000, reco reset >> 400, -10]
+		Bal: 390
+		"""
+		sle = [
+			frappe._dict(
+				name="Flask Item",
+				actual_qty=0, qty_after_transaction=1000,
+				warehouse="WH 1",
+				posting_date="2021-12-01", voucher_type="Stock Reconciliation",
+				voucher_no="002",
+				has_serial_no=False, serial_no=None
+			),
+			frappe._dict(
+				name="Flask Item",
+				actual_qty=0, qty_after_transaction=400,
+				warehouse="WH 1",
+				posting_date="2021-12-02", voucher_type="Stock Reconciliation",
+				voucher_no="003",
+				has_serial_no=False, serial_no=None
+			),
+			frappe._dict(
+				name="Flask Item",
+				actual_qty=(-10), qty_after_transaction=390,
+				warehouse="WH 1",
+				posting_date="2021-12-03", voucher_type="Stock Entry",
+				voucher_no="003",
+				has_serial_no=False, serial_no=None
+			)
+		]
+		slots = FIFOSlots(self.filters, sle).generate()
+
+		result = slots["Flask Item"]
+		queue = result["fifo_queue"]
+
+		self.assertEqual(result["qty_after_transaction"], result["total_qty"])
+		self.assertEqual(result["total_qty"], 390.0)
+		self.assertEqual(queue[0][0], 390.0)
+
+	def test_sequential_stock_reco_different_warehouse(self):
+		"""
+		Ledger:
+		WH	| Voucher | Qty
+		-------------------
+		WH1 | Reco	  | 1000
+		WH2 | Reco	  | 400
+		WH1 | SE	  | -10
+
+		Bal: WH1 bal + WH2 bal = 990 + 400 = 1390
+		"""
+		sle = [
+			frappe._dict(
+				name="Flask Item",
+				actual_qty=0, qty_after_transaction=1000,
+				warehouse="WH 1",
+				posting_date="2021-12-01", voucher_type="Stock Reconciliation",
+				voucher_no="002",
+				has_serial_no=False, serial_no=None
+			),
+			frappe._dict(
+				name="Flask Item",
+				actual_qty=0, qty_after_transaction=400,
+				warehouse="WH 2",
+				posting_date="2021-12-02", voucher_type="Stock Reconciliation",
+				voucher_no="003",
+				has_serial_no=False, serial_no=None
+			),
+			frappe._dict(
+				name="Flask Item",
+				actual_qty=(-10), qty_after_transaction=990,
+				warehouse="WH 1",
+				posting_date="2021-12-03", voucher_type="Stock Entry",
+				voucher_no="004",
+				has_serial_no=False, serial_no=None
+			)
+		]
+
+		item_wise_slots, item_wh_wise_slots = generate_item_and_item_wh_wise_slots(
+			filters=self.filters,sle=sle
+		)
+
+		# test without 'show_warehouse_wise_stock'
+		item_result = item_wise_slots["Flask Item"]
+		queue = item_result["fifo_queue"]
+
+		self.assertEqual(item_result["qty_after_transaction"], item_result["total_qty"])
+		self.assertEqual(item_result["total_qty"], 1390.0)
+		self.assertEqual(queue[0][0], 990.0)
+		self.assertEqual(queue[1][0], 400.0)
+
+		# test with 'show_warehouse_wise_stock' checked
+		item_wh_balances = [item_wh_wise_slots.get(i).get("qty_after_transaction") for i in item_wh_wise_slots]
+		self.assertEqual(sum(item_wh_balances), item_result["qty_after_transaction"])
+
+def generate_item_and_item_wh_wise_slots(filters, sle):
+	"Return results with and without 'show_warehouse_wise_stock'"
+	item_wise_slots = FIFOSlots(filters, sle).generate()
+
+	filters.show_warehouse_wise_stock = True
+	item_wh_wise_slots = FIFOSlots(filters, sle).generate()
+	filters.show_warehouse_wise_stock = False
+
+	return item_wise_slots, item_wh_wise_slots