mirror of
https://github.com/frappe/erpnext.git
synced 2026-09-01 23:53:21 +00:00
fix(pos): return raw Item Group names from get_item_groups (double-escape regression)
The Postgres-portability change moved the POS item-group filters to the query builder
(item.item_group.isin(...)) and frappe.get_all(["name","in",...]), which escape values
once. get_item_groups() still pre-escaped each name with frappe.db.escape(), so the
names were escaped TWICE -> `item_group IN ('''Products''')`, matching nothing. Any POS
Profile that restricts item groups returned ZERO items, on both MariaDB and Postgres.
Return raw names; the parameterized callers escape them correctly. (get_parent_item_group
also returned the quoted literal before this fix.) Add a regression test: a POS Profile
restricted to an item group must still surface that group's items — it returns 0 before
the fix and passes after, on both engines.
This commit is contained in:
@@ -242,15 +242,18 @@ def get_item_groups(pos_profile):
|
|||||||
for data in pos_profile.get("item_groups"):
|
for data in pos_profile.get("item_groups"):
|
||||||
item_groups.extend(
|
item_groups.extend(
|
||||||
[
|
[
|
||||||
"%s" % frappe.db.escape(d.name)
|
d.name
|
||||||
for d in get_child_nodes("Item Group", data.item_group)
|
for d in get_child_nodes("Item Group", data.item_group)
|
||||||
if not permitted_item_groups or d.name in permitted_item_groups
|
if not permitted_item_groups or d.name in permitted_item_groups
|
||||||
]
|
]
|
||||||
)
|
)
|
||||||
|
|
||||||
if not item_groups and permitted_item_groups:
|
if not item_groups and permitted_item_groups:
|
||||||
item_groups = ["%s" % frappe.db.escape(d) for d in permitted_item_groups]
|
item_groups = list(permitted_item_groups)
|
||||||
|
|
||||||
|
# Return raw Item Group names; the callers parameterize them via the query builder
|
||||||
|
# (item_group.isin(...)) / frappe.get_all, which escapes them once. Pre-escaping here would
|
||||||
|
# double-escape (item_group IN ('''X''')) and match nothing.
|
||||||
return list(set(item_groups))
|
return list(set(item_groups))
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -102,6 +102,32 @@ class TestPointOfSaleGetItems(ERPNextTestSuite):
|
|||||||
item_codes = self._get_item_codes(self.item_code)
|
item_codes = self._get_item_codes(self.item_code)
|
||||||
self.assertNotIn(self.item_code, item_codes)
|
self.assertNotIn(self.item_code, item_codes)
|
||||||
|
|
||||||
|
def test_pos_profile_item_group_restriction_returns_items(self):
|
||||||
|
# get_item_groups() returns Item Group names that the callers parameterize via
|
||||||
|
# item.item_group.isin(...) / frappe.get_all, which escapes them once. The names must be RAW;
|
||||||
|
# pre-escaping them double-escaped to `item_group IN ('''X''')`, so a POS Profile restricting
|
||||||
|
# item groups matched nothing and returned ZERO items on both engines.
|
||||||
|
from erpnext.accounts.doctype.pos_profile.pos_profile import get_item_groups
|
||||||
|
|
||||||
|
restricted = make_pos_profile(name="_Test POS Profile IG Restricted")
|
||||||
|
restricted.append("item_groups", {"item_group": self.item_group})
|
||||||
|
restricted.save()
|
||||||
|
|
||||||
|
# the helper must hand back the real (raw) group name, not an escaped literal
|
||||||
|
self.assertIn(self.item_group, get_item_groups(restricted.name))
|
||||||
|
|
||||||
|
# end-to-end: the restricted profile must still surface its in-group items
|
||||||
|
result = get_items(
|
||||||
|
start=0,
|
||||||
|
page_length=100,
|
||||||
|
price_list="Standard Selling",
|
||||||
|
item_group=self.item_group,
|
||||||
|
pos_profile=restricted.name,
|
||||||
|
search_term="",
|
||||||
|
)
|
||||||
|
items = result["items"] if isinstance(result, dict) else result
|
||||||
|
self.assertIn(self.item_code, [row.get("item_code") for row in items])
|
||||||
|
|
||||||
def test_non_sales_item_is_excluded(self):
|
def test_non_sales_item_is_excluded(self):
|
||||||
# is_sales_item == 1 is part of the converted WHERE clause.
|
# is_sales_item == 1 is part of the converted WHERE clause.
|
||||||
frappe.db.set_value("Item", self.item_code, "is_sales_item", 0)
|
frappe.db.set_value("Item", self.item_code, "is_sales_item", 0)
|
||||||
|
|||||||
Reference in New Issue
Block a user