fix: validate POS Settings invoice and search fields on the server (backport #58611) (#58612)

erpnext/accounts/doctype/pos_search_fields/pos_search_fields.json

@@ -12,20 +12,22 @@
   {
    "fieldname": "fieldname",
    "fieldtype": "Data",
-   "hidden": 1,
-   "label": "Fieldname"
+   "in_list_view": 1,
+   "label": "Fieldname",
+   "read_only": 1
   },
   {
    "fieldname": "field",
    "fieldtype": "Select",
    "in_list_view": 1,
-   "label": "Field"
+   "label": "Field",
+   "reqd": 1
   }
  ],
  "index_web_pages_for_search": 1,
  "istable": 1,
  "links": [],
- "modified": "2021-04-21 11:12:54.632093",
+ "modified": "2026-08-31 20:41:12.000000",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "POS Search Fields",
@@ -34,4 +36,4 @@
  "sort_field": "modified",
  "sort_order": "DESC",
  "track_changes": 1
-}
+}

erpnext/accounts/doctype/pos_settings/pos_settings.js

@@ -1,40 +1,9 @@
 // Copyright (c) 2017, Frappe Technologies Pvt. Ltd. and contributors
 // For license information, please see license.txt
 
-let search_fields_datatypes = [
-	"Data",
-	"Link",
-	"Dynamic Link",
-	"Long Text",
-	"Select",
-	"Small Text",
-	"Text",
-	"Text Editor",
-];
-let do_not_include_fields = [
-	"naming_series",
-	"item_code",
-	"item_name",
-	"stock_uom",
-	"asset_naming_series",
-	"default_material_request_type",
-	"valuation_method",
-	"warranty_period",
-	"weight_uom",
-	"batch_number_series",
-	"serial_no_series",
-	"purchase_uom",
-	"customs_tariff_number",
-	"sales_uom",
-	"deferred_revenue_account",
-	"deferred_expense_account",
-	"quality_inspection_template",
-	"route",
-	"slideshow",
-	"website_image_alt",
-	"thumbnail",
-	"web_long_description",
-];
+function is_valid_invoice_field(df) {
+	return frappe.model.no_value_type.indexOf(df.fieldtype) === -1 || df.fieldtype === "Button";
+}
 
 frappe.ui.form.on("POS Settings", {
 	onload: function (frm) {
@@ -44,57 +13,46 @@ frappe.ui.form.on("POS Settings", {
 
 	get_invoice_fields: function (frm) {
 		frappe.model.with_doctype("POS Invoice", () => {
-			var fields = $.map(frappe.get_doc("DocType", "POS Invoice").fields, function (d) {
-				if (
-					frappe.model.no_value_type.indexOf(d.fieldtype) === -1 ||
-					["Button"].includes(d.fieldtype)
-				) {
-					return { label: d.label + " (" + d.fieldtype + ")", value: d.fieldname };
-				} else {
-					return null;
-				}
-			});
+			const fields = frappe.get_doc("DocType", "POS Invoice").fields.filter(is_valid_invoice_field);
 
 			frm.fields_dict.invoice_fields.grid.update_docfield_property(
 				"fieldname",
 				"options",
-				[""].concat(fields)
+				[""].concat(
+					fields.map((df) => {
+						return { label: `${df.label} (${df.fieldtype})`, value: df.fieldname };
+					})
+				)
 			);
 		});
 	},
 
 	add_search_options: function (frm) {
-		frappe.model.with_doctype("Item", () => {
-			var fields = $.map(frappe.get_doc("DocType", "Item").fields, function (d) {
-				if (
-					search_fields_datatypes.includes(d.fieldtype) &&
-					!do_not_include_fields.includes(d.fieldname)
-				) {
-					return [d.label];
-				} else {
-					return null;
-				}
-			});
+		frappe.call({
+			method: "erpnext.accounts.doctype.pos_settings.pos_settings.get_pos_search_field_options",
+			callback: ({ message }) => {
+				const fields = message || [];
 
-			fields.unshift("");
-			frm.fields_dict.pos_search_fields.grid.update_docfield_property("field", "options", fields);
+				frm.searchable_item_fields = Object.fromEntries(
+					fields.map((df) => [df.option, df.fieldname])
+				);
+
+				frm.fields_dict.pos_search_fields.grid.update_docfield_property(
+					"field",
+					"options",
+					[""].concat(fields.map((df) => df.option))
+				);
+			},
 		});
 	},
 });
 
 frappe.ui.form.on("POS Search Fields", {
 	field: function (frm, doctype, name) {
-		var doc = frappe.get_doc(doctype, name);
-		var df = $.map(frappe.get_doc("DocType", "Item").fields, function (d) {
-			if (doc.field == d.label && search_fields_datatypes.includes(d.fieldtype)) {
-				return d;
-			} else {
-				return null;
-			}
-		})[0];
+		const doc = frappe.get_doc(doctype, name);
 
-		doc.fieldname = df.fieldname;
-		frm.refresh_field("fields");
+		doc.fieldname = frm.searchable_item_fields?.[doc.field] || "";
+		frm.refresh_field("pos_search_fields");
 	},
 });
 
@@ -110,6 +68,6 @@ frappe.ui.form.on("POS Field", {
 		doc.options = df.options;
 		doc.fieldtype = df.fieldtype;
 		doc.default_value = df.default;
-		frm.refresh_field("fields");
+		frm.refresh_field("invoice_fields");
 	},
 });

erpnext/accounts/doctype/pos_settings/pos_settings.py

@@ -1,9 +1,50 @@
 # Copyright (c) 2017, Frappe Technologies Pvt. Ltd. and contributors
 # For license information, please see license.txt
 
+from collections import Counter
 
+import frappe
+from frappe import _
+from frappe.model import no_value_fields
 from frappe.model.document import Document
 
+SEARCH_FIELD_TYPES = (
+	"Data",
+	"Link",
+	"Dynamic Link",
+	"Long Text",
+	"Select",
+	"Small Text",
+	"Text",
+	"Text Editor",
+)
+
+# Item fields that are of a searchable fieldtype, but are not meaningful to search a POS item by
+DO_NOT_INCLUDE_FIELDS = (
+	"naming_series",
+	"item_code",
+	"item_name",
+	"stock_uom",
+	"asset_naming_series",
+	"default_material_request_type",
+	"valuation_method",
+	"warranty_period",
+	"weight_uom",
+	"batch_number_series",
+	"serial_no_series",
+	"purchase_uom",
+	"customs_tariff_number",
+	"sales_uom",
+	"deferred_revenue_account",
+	"deferred_expense_account",
+	"quality_inspection_template",
+	"route",
+	"slideshow",
+	"website_image_alt",
+	"thumbnail",
+	"web_long_description",
+)
+
 
 class POSSettings(Document):
 	# begin: auto-generated types
@@ -22,4 +63,95 @@ class POSSettings(Document):
 	# end: auto-generated types
 
 	def validate(self):
-		pass
+		self.validate_duplicate_invoice_fields()
+		self.validate_invoice_fields()
+		self.validate_duplicate_pos_search_fields()
+		self.validate_pos_search_fields()
+
+	def validate_duplicate_invoice_fields(self):
+		fieldnames = [field.fieldname for field in self.invoice_fields]
+
+		for fieldname, count in Counter(fieldnames).items():
+			if count > 1:
+				frappe.throw(
+					title=_("Duplicate POS Fields"), msg=_("'{0}' has been already added.").format(fieldname)
+				)
+
+	def validate_invoice_fields(self):
+		# the POS screen only ever creates a POS Invoice
+		meta = frappe.get_meta("POS Invoice")
+
+		for field in self.invoice_fields:
+			df = meta.get_field(field.fieldname)
+
+			if not df or not is_valid_invoice_field(df):
+				frappe.throw(
+					title=_("Invalid POS Field"),
+					msg=_("Row #{0}: '{1}' is not a valid field of {2}.").format(
+						field.idx, frappe.bold(field.fieldname or ""), frappe.bold(_("POS Invoice"))
+					),
+				)
+
+			# read only in the form, so keep them in sync with the invoice
+			field.label = df.label
+			field.fieldtype = df.fieldtype
+			field.options = df.options
+
+	def validate_duplicate_pos_search_fields(self):
+		fieldnames = [field.fieldname for field in self.pos_search_fields]
+
+		for fieldname, count in Counter(fieldnames).items():
+			if count > 1:
+				frappe.throw(
+					title=_("Duplicate POS Search Fields"),
+					msg=_("'{0}' has been already added.").format(fieldname),
+				)
+
+	def validate_pos_search_fields(self):
+		searchable_fields = {df.fieldname: df for df in get_searchable_item_fields()}
+
+		for field in self.pos_search_fields:
+			df = searchable_fields.get(field.fieldname)
+
+			if not df:
+				frappe.throw(
+					title=_("Invalid POS Search Field"),
+					msg=_("Row #{0}: '{1}' cannot be used to search items.").format(
+						field.idx, frappe.bold(field.fieldname or "")
+					),
+				)
+
+			if field.field != get_search_field_option(df):
+				frappe.throw(
+					title=_("Invalid POS Search Field"),
+					msg=_("Row #{0}: '{1}' does not match {2}.").format(
+						field.idx, frappe.bold(field.field or ""), frappe.bold(df.fieldname)
+					),
+				)
+
+
+def is_valid_invoice_field(df):
+	return df.fieldtype not in no_value_fields or df.fieldtype == "Button"
+
+
+def get_searchable_item_fields():
+	return [
+		df
+		for df in frappe.get_meta("Item").fields
+		if df.fieldtype in SEARCH_FIELD_TYPES and df.fieldname not in DO_NOT_INCLUDE_FIELDS
+	]
+
+
+def get_search_field_option(df):
+	# the fieldname keeps the option unique, two Item fields can share a label
+	return f"{df.label} ({df.fieldname})"
+
+
+@frappe.whitelist()
+def get_pos_search_field_options():
+	frappe.has_permission("POS Settings", throw=True)
+
+	return [
+		{"option": get_search_field_option(df), "fieldname": df.fieldname}
+		for df in get_searchable_item_fields()
+	]

erpnext/accounts/doctype/pos_settings/test_pos_settings.py

@@ -3,6 +3,119 @@
 
 import unittest
 
+import frappe
+
+from erpnext.patches.v16_0.append_fieldname_to_pos_search_fields import execute as append_fieldname
+
 
 class TestPOSSettings(unittest.TestCase):
-	pass
+	def setUp(self):
+		self.settings = frappe.get_single("POS Settings")
+		self.settings.invoice_fields = []
+		self.settings.pos_search_fields = []
+
+	def tearDown(self):
+		frappe.db.rollback()
+
+	def assertInvalid(self, message):
+		with self.assertRaises(frappe.ValidationError) as context:
+			self.settings.save()
+
+		self.assertIn(message, str(context.exception))
+
+	def test_duplicate_invoice_field_is_not_allowed(self):
+		self.settings.append("invoice_fields", {"fieldname": "customer"})
+		self.settings.append("invoice_fields", {"fieldname": "customer"})
+		self.assertInvalid("'customer' has been already added.")
+
+	def test_unknown_invoice_field_is_not_allowed(self):
+		self.settings.append("invoice_fields", {"fieldname": "not_a_field"})
+		self.assertInvalid("is not a valid field of")
+
+	def test_layout_invoice_field_is_not_allowed(self):
+		self.settings.append("invoice_fields", {"fieldname": "accounting_dimensions_section"})
+		self.assertInvalid("is not a valid field of")
+
+	def test_invoice_field_properties_are_set_from_the_invoice(self):
+		self.settings.append(
+			"invoice_fields", {"fieldname": "customer", "label": "Tampered", "fieldtype": "Data"}
+		)
+		self.settings.save()
+
+		field = self.settings.invoice_fields[0]
+		self.assertEqual(field.label, "Customer")
+		self.assertEqual(field.fieldtype, "Link")
+		self.assertEqual(field.options, "Customer")
+
+	def test_searchable_item_field_is_allowed(self):
+		self.settings.append(
+			"pos_search_fields", {"field": "Description (description)", "fieldname": "description"}
+		)
+		self.settings.save()
+
+		self.assertEqual(self.settings.pos_search_fields[0].fieldname, "description")
+
+	def test_excluded_search_field_is_not_allowed(self):
+		self.settings.append(
+			"pos_search_fields", {"field": "Item Name (item_name)", "fieldname": "item_name"}
+		)
+		self.assertInvalid("cannot be used to search items")
+
+	def test_search_field_of_unsearchable_type_is_not_allowed(self):
+		# maintain stock is a Check field
+		self.settings.append(
+			"pos_search_fields", {"field": "Maintain Stock (is_stock_item)", "fieldname": "is_stock_item"}
+		)
+		self.assertInvalid("cannot be used to search items")
+
+	def test_unknown_search_field_is_not_allowed(self):
+		self.settings.append(
+			"pos_search_fields", {"field": "Nope (not_an_item_field)", "fieldname": "not_an_item_field"}
+		)
+		self.assertInvalid("cannot be used to search items")
+
+	def test_search_field_without_a_fieldname_is_not_allowed(self):
+		# the form fills the fieldname in, it cannot be picked on its own
+		self.settings.append("pos_search_fields", {"field": "Description (description)"})
+		self.assertInvalid("cannot be used to search items")
+
+	def test_search_field_option_must_match_its_fieldname(self):
+		self.settings.append("pos_search_fields", {"field": "Brand (brand)", "fieldname": "description"})
+		self.assertInvalid("does not match")
+
+	def test_bare_label_is_not_accepted_as_a_search_field(self):
+		# the stored option carries the fieldname, the patch backfills older rows
+		self.settings.append("pos_search_fields", {"field": "Description", "fieldname": "description"})
+		self.assertInvalid("does not match")
+
+	def test_duplicate_search_fields_are_not_allowed(self):
+		for _ in range(2):
+			self.settings.append(
+				"pos_search_fields", {"field": "Description (description)", "fieldname": "description"}
+			)
+
+		self.assertInvalid("has been already added")
+
+	def test_patch_appends_the_fieldname_to_a_legacy_search_field(self):
+		self.settings.append(
+			"pos_search_fields", {"field": "Description (description)", "fieldname": "description"}
+		)
+		self.settings.save()
+
+		row = self.settings.pos_search_fields[0].name
+		frappe.db.set_value("POS Search Fields", row, "field", "Description", update_modified=False)
+
+		append_fieldname()
+
+		self.assertEqual(frappe.db.get_value("POS Search Fields", row, "field"), "Description (description)")
+
+	def test_patch_leaves_an_already_migrated_search_field_alone(self):
+		self.settings.append(
+			"pos_search_fields", {"field": "Description (description)", "fieldname": "description"}
+		)
+		self.settings.save()
+
+		append_fieldname()
+
+		row = self.settings.pos_search_fields[0].name
+		self.assertEqual(frappe.db.get_value("POS Search Fields", row, "field"), "Description (description)")

erpnext/patches.txt

@@ -449,3 +449,4 @@ erpnext.patches.v16_0.backfill_repost_accounting_ledger_status
 erpnext.patches.v16_0.merge_seeded_item_group_root
 erpnext.patches.v16_0.repair_work_order_material_transfer
 erpnext.patches.v16_0.remove_frappe_crm_custom_fields
+erpnext.patches.v16_0.append_fieldname_to_pos_search_fields

erpnext/patches/v16_0/append_fieldname_to_pos_search_fields.py

@@ -0,0 +1,20 @@
+import frappe
+
+
+def execute():
+	rows = frappe.get_all(
+		"POS Search Fields", filters={"parent": "POS Settings"}, fields=["name", "field", "fieldname"]
+	)
+
+	for row in rows:
+		# the row used to hold the label alone, it now holds "Label (fieldname)"
+		if not (row.field and row.fieldname) or row.field.endswith(f"({row.fieldname})"):
+			continue
+
+		frappe.db.set_value(
+			"POS Search Fields",
+			row.name,
+			"field",
+			f"{row.field} ({row.fieldname})",
+			update_modified=False,
+		)