Merge pull request #56209 from mihir-kandoi/pg-stock-batch-report-item-attribute

fix(stock): Available Batch report GROUP BY + ItemAttribute raw SQL→qb (Postgres)

erpnext/stock/doctype/item_attribute/item_attribute.py

@@ -66,18 +66,15 @@ class ItemAttribute(Document):
 		attributes_list = [d.attribute_value for d in self.item_attribute_values]
 
 		# Get Item Variant Attribute details of variant items
-		items = frappe.db.sql(
-			"""
-			select
-				i.name, iva.attribute_value as value
-			from
-				`tabItem Variant Attribute` iva, `tabItem` i
-			where
-				iva.attribute = %(attribute)s
-				and iva.parent = i.name and
-				i.variant_of is not null and i.variant_of != ''""",
-			{"attribute": self.name},
-			as_dict=1,
+		iva = frappe.qb.DocType("Item Variant Attribute")
+		i = frappe.qb.DocType("Item")
+		items = (
+			frappe.qb.from_(iva)
+			.inner_join(i)
+			.on(iva.parent == i.name)
+			.select(i.name, iva.attribute_value.as_("value"))
+			.where((iva.attribute == self.name) & i.variant_of.isnotnull() & (i.variant_of != ""))
+			.run(as_dict=1)
 		)
 
 		for item in items:

erpnext/stock/doctype/item_attribute/test_item_attribute.py

@@ -30,3 +30,22 @@ class TestItemAttribute(ERPNextTestSuite):
 
 		item_attribute.increment = 0.5
 		item_attribute.save()
+
+	def test_validate_existing_items_finds_variants(self):
+		# validate_exising_items() joins Item Variant Attribute to Item to find variants using this
+		# attribute. Exercises the converted query builder version on both engines and asserts it
+		# finds the variant (the raise only fires if the query returned the variant row).
+		from erpnext.controllers.item_variant import InvalidItemAttributeValueError, create_variant
+
+		frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1)
+		variant = create_variant("_Test Variant Item", {"Test Size": "Large"})
+		variant.save()
+		self.addCleanup(frappe.delete_doc_if_exists, "Item", "_Test Variant Item-L", force=1)
+
+		attribute = frappe.get_doc("Item Attribute", "Test Size")
+		attribute.item_attribute_values = []
+		frappe.flags.attribute_values = None
+
+		# "Large" is no longer a permitted value, so the variant found by validate_exising_items
+		# is invalid; the save must abort (and so never persists the cleared values).
+		self.assertRaises(InvalidItemAttributeValueError, attribute.save)

erpnext/stock/report/available_batch_report/available_batch_report.py

@@ -106,7 +106,9 @@ def get_batchwise_data_from_stock_ledger(filters):
 			Sum(table.actual_qty).as_("balance_qty"),
 		)
 		.where(table.is_cancelled == 0)
-		.groupby(table.batch_no, table.item_code, table.warehouse)
+		# batch.expiry_date comes from the Batch table; postgres requires its PK in the GROUP BY for
+		# it to be selectable. batch.name is 1:1 with the grouped batch_no, so groups are unchanged.
+		.groupby(table.batch_no, table.item_code, table.warehouse, batch.name)
 	)
 
 	query = get_query_based_on_filters(query, batch, table, filters)
@@ -137,7 +139,10 @@ def get_batchwise_data_from_serial_batch_bundle(batchwise_data, filters):
 			Sum(ch_table.qty).as_("balance_qty"),
 		)
 		.where((table.is_cancelled == 0) & (table.docstatus == 1))
-		.groupby(ch_table.batch_no, table.item_code, ch_table.warehouse)
+		# Group by the same (SLE) warehouse that is selected -- the original grouped by
+		# ch_table.warehouse while selecting table.warehouse, which postgres rejects. Also group by
+		# the Batch PK so batch.expiry_date is selectable (1:1 with the grouped batch_no).
+		.groupby(ch_table.batch_no, table.item_code, table.warehouse, batch.name)
 	)
 
 	query = get_query_based_on_filters(query, batch, table, filters)

erpnext/stock/report/available_batch_report/test_available_batch_report.py

@@ -0,0 +1,49 @@
+# Copyright (c) 2024, Frappe Technologies Pvt. Ltd. and contributors
+# For license information, please see license.txt
+
+import frappe
+from frappe.utils import today
+
+from erpnext.tests.utils import ERPNextTestSuite
+
+
+class TestAvailableBatchReport(ERPNextTestSuite):
+	@staticmethod
+	def _cancel_and_delete_stock_entry(name):
+		if not frappe.db.exists("Stock Entry", name):
+			return
+		doc = frappe.get_doc("Stock Entry", name)
+		if doc.docstatus == 1:
+			doc.cancel()
+		frappe.delete_doc("Stock Entry", name, force=1)
+
+	def test_report_runs_and_lists_batch_qty(self):
+		# The report selects Batch columns (expiry_date, and item_name when show_item_name is set)
+		# while grouping by SLE columns; the Batch PK must be in the GROUP BY for the report to run
+		# on Postgres. show_item_name=1 forces the extra Batch column to be selected.
+		from erpnext.stock.doctype.item.test_item import make_item
+		from erpnext.stock.doctype.serial_and_batch_bundle.test_serial_and_batch_bundle import (
+			get_batch_from_bundle,
+		)
+		from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
+		from erpnext.stock.report.available_batch_report.available_batch_report import execute
+
+		item = make_item(
+			"_Test Available Batch Report Item",
+			{"has_batch_no": 1, "create_new_batch": 1, "is_stock_item": 1},
+		).name
+		se = make_stock_entry(
+			item_code=item, target="_Test Warehouse - _TC", qty=7, basic_rate=10, purpose="Material Receipt"
+		)
+		# make_item is idempotent (returns the existing item), but each receipt stacks a new batch,
+		# so cancel+delete the stock entry to keep repeated runs clean.
+		self.addCleanup(self._cancel_and_delete_stock_entry, se.name)
+		batch_no = get_batch_from_bundle(se.items[0].serial_and_batch_bundle)
+
+		filters = frappe._dict(to_date=today(), item_code=item, show_item_name=1)
+		columns, data = execute(filters)
+
+		self.assertTrue(columns)
+		row = next((d for d in data if d.batch_no == batch_no), None)
+		self.assertIsNotNone(row)
+		self.assertEqual(row.balance_qty, 7)