nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-08-13 22:51:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(stock): make the batch-number picker Postgres-correct

Browse Source 
The batch-number link picker (get_batch_no) had two Postgres-only defects in
both of its query builders (get_batches_from_stock_ledger_entries and
get_batches_from_serial_and_batch_bundle):

1. GROUP BY. They group by Stock Ledger Entry / Serial-and-Batch-Entry columns
   while selecting un-aggregated Batch-master columns (manufacturing_date,
   expiry_date, search fields). PostgreSQL only accepts that when the Batch
   primary key is in the GROUP BY, so the picker raised GroupingError. Adding
   batch_table.name (equal to the grouped batch_no via the join) keeps the
   group count - and the MariaDB result - unchanged while making it valid.

2. CONCAT over nullable dates. "MFG-"/"EXP-" labels were built with
   Concat("MFG-", manufacturing_date). When the date is NULL, MariaDB CONCAT
   returns NULL but Postgres CONCAT drops the NULL and yields a bare "MFG-"/
   "EXP-". Guard each with Case().when(date.isnotnull(), ...) so a missing date
   is NULL on both engines (matching MariaDB, fixing Postgres).

Both leave MariaDB output unchanged. test_get_batch_no_search_returns_batches
exercises both builders directly and asserts no bare "MFG-"/"EXP-" leaks;
reverting either fix makes it fail on Postgres.
This commit is contained in:
Mihir Kandoi
2026-06-23 07:35:36 +05:30
parent ff737df55f
commit dc4eee49cc
2 changed files with 60 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
erpnext/controllers/queries.py
View File

@@ -591,7 +591,7 @@ def get_batches_from_stock_ledger_entries(searchfields, txt, filters, start=0, p
& (batch_table.disabled == 0)
& (stock_ledger_entry.batch_no.isnotnull())
)
.groupby(stock_ledger_entry.batch_no, stock_ledger_entry.warehouse)
.groupby(stock_ledger_entry.batch_no, stock_ledger_entry.warehouse, batch_table.name)
.having(Sum(stock_ledger_entry.actual_qty) != 0)
.offset(start)
.limit(page_len)
@@ -608,8 +608,12 @@ def get_batches_from_stock_ledger_entries(searchfields, txt, filters, start=0, p
query = query.where((batch_table.expiry_date >= expiry_date) | (batch_table.expiry_date.isnull()))
query = query.select(
Concat("MFG-", batch_table.manufacturing_date).as_("manufacturing_date"),
Concat("EXP-", batch_table.expiry_date).as_("expiry_date"),
Case()
.when(batch_table.manufacturing_date.isnotnull(), Concat("MFG-", batch_table.manufacturing_date))
.as_("manufacturing_date"),
Case()
.when(batch_table.expiry_date.isnotnull(), Concat("EXP-", batch_table.expiry_date))
.as_("expiry_date"),
)
if filters.get("warehouse"):
@@ -651,7 +655,7 @@ def get_batches_from_serial_and_batch_bundle(searchfields, txt, filters, start=0
& (batch_table.disabled == 0)
& (stock_ledger_entry.serial_and_batch_bundle.isnotnull())
)
.groupby(bundle.batch_no, bundle.warehouse)
.groupby(bundle.batch_no, bundle.warehouse, batch_table.name)
.having(Sum(bundle.qty) != 0)
.offset(start)
.limit(page_len)
@@ -670,8 +674,12 @@ def get_batches_from_serial_and_batch_bundle(searchfields, txt, filters, start=0
)
bundle_query = bundle_query.select(
Concat("MFG-", batch_table.manufacturing_date),
Concat("EXP-", batch_table.expiry_date),
Case()
.when(batch_table.manufacturing_date.isnotnull(), Concat("MFG-", batch_table.manufacturing_date))
.as_("manufacturing_date"),
Case()
.when(batch_table.expiry_date.isnotnull(), Concat("EXP-", batch_table.expiry_date))
.as_("expiry_date"),
)
if filters.get("warehouse"):

46
erpnext/stock/doctype/batch/test_batch.py
View File

@@ -386,6 +386,52 @@ class TestBatch(ERPNextTestSuite):
self.assertEqual(get_batch_qty("batch a", "_Test Warehouse - _TC"), 90)
def test_get_batch_no_search_returns_batches(self):
"""The batch-number picker must run on every engine.
Both query builders group by Stock Ledger Entry / Serial-and-Batch-Entry
columns while selecting un-aggregated Batch-master columns; PostgreSQL only
accepts that when the Batch primary key is in the GROUP BY, so the picker
errors there without it.
"""
from erpnext.controllers.queries import (
get_batch_no,
get_batches_from_serial_and_batch_bundle,
get_batches_from_stock_ledger_entries,
)
self.make_batch_item("ITEM-BATCH-PICKER")
self.make_new_batch_and_entry("ITEM-BATCH-PICKER", "batch picker a", "_Test Warehouse - _TC")
self.make_new_batch_and_entry("ITEM-BATCH-PICKER", "batch picker b", "_Test Warehouse - _TC")
searchfields = frappe.get_meta("Batch").get_search_fields()
filters = {"item_code": "ITEM-BATCH-PICKER", "warehouse": "_Test Warehouse - _TC"}
# Exercise both query builders directly so each GROUP BY is covered regardless
# of which path holds the data: PostgreSQL validates the GROUP BY even when no
# rows match, so a missing Batch primary key raises GroupingError here.
get_batches_from_stock_ledger_entries(searchfields, "", filters)
get_batches_from_serial_and_batch_bundle(searchfields, "", filters)
result = get_batch_no(
doctype="Batch",
txt="",
searchfield="name",
start=0,
page_len=20,
filters=filters,
)
returned = {row[0] for row in result}
self.assertIn("batch picker a", returned)
self.assertIn("batch picker b", returned)
# These batches have no manufacturing/expiry date. MariaDB CONCAT('MFG-', NULL)
# is NULL, but Postgres CONCAT drops the NULL and would surface a bare "MFG-"/
# "EXP-"; the null-guarded select must keep both engines free of that artifact.
flat = [value for row in result for value in row]
self.assertNotIn("MFG-", flat)
self.assertNotIn("EXP-", flat)
def test_ignore_reserved_qty(self):
from erpnext.selling.doctype.sales_order.mapper import create_pick_list
from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order