Merge branch 'develop' into fix-pos-issues-again
diff --git a/erpnext/accounts/doctype/pos_invoice/pos_invoice.json b/erpnext/accounts/doctype/pos_invoice/pos_invoice.json
index 0c6e7ed..b850027 100644
--- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.json
+++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.json
@@ -264,7 +264,6 @@
    "print_hide": 1
   },
   {
-   "allow_on_submit": 1,
    "default": "0",
    "fieldname": "is_return",
    "fieldtype": "Check",
@@ -1573,7 +1572,7 @@
  "icon": "fa fa-file-text",
  "is_submittable": 1,
  "links": [],
- "modified": "2021-10-05 12:11:53.871828",
+ "modified": "2022-03-22 13:00:24.166684",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "POS Invoice",
@@ -1623,6 +1622,7 @@
  "show_name_in_global_search": 1,
  "sort_field": "modified",
  "sort_order": "DESC",
+ "states": [],
  "timeline_field": "customer",
  "title_field": "title",
  "track_changes": 1,
diff --git a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py
index d782cc2..275eeb3 100644
--- a/erpnext/accounts/doctype/pos_invoice/pos_invoice.py
+++ b/erpnext/accounts/doctype/pos_invoice/pos_invoice.py
@@ -16,7 +16,11 @@
 )
 from erpnext.accounts.party import get_due_date, get_party_account
 from erpnext.stock.doctype.batch.batch import get_batch_qty, get_pos_reserved_batch_qty
-from erpnext.stock.doctype.serial_no.serial_no import get_pos_reserved_serial_nos, get_serial_nos
+from erpnext.stock.doctype.serial_no.serial_no import (
+	get_delivered_serial_nos,
+	get_pos_reserved_serial_nos,
+	get_serial_nos,
+)
 
 
 class POSInvoice(SalesInvoice):
@@ -145,12 +149,7 @@
 						.format(item.idx, bold_invalid_batch_no, bold_item_name, bold_extra_batch_qty_needed), title=_("Item Unavailable"))
 
 	def validate_delivered_serial_nos(self, item):
-		serial_nos = get_serial_nos(item.serial_no)
-		delivered_serial_nos = frappe.db.get_list('Serial No', {
-			'item_code': item.item_code,
-			'name': ['in', serial_nos],
-			'sales_invoice': ['is', 'set']
-		}, pluck='name')
+		delivered_serial_nos = get_delivered_serial_nos(item.serial_no)
 
 		if delivered_serial_nos:
 			bold_delivered_serial_nos = frappe.bold(', '.join(delivered_serial_nos))
@@ -172,10 +171,14 @@
 			frappe.throw(error_msg, title=_("Invalid Item"), as_list=True)
 
 	def validate_stock_availablility(self):
+		if self.is_return:
+			return
+
+		if self.docstatus.is_draft() and not frappe.db.get_value('POS Profile', self.pos_profile, 'validate_stock_on_save'):
+			return
+
 		from erpnext.stock.stock_ledger import is_negative_stock_allowed
 
-		if self.is_return or self.docstatus != 1:
-			return
 		for d in self.get('items'):
 			is_service_item = not (frappe.db.get_value('Item', d.get('item_code'), 'is_stock_item'))
 			if is_service_item:
diff --git a/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py b/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py
index cf8affd..a76ae12 100644
--- a/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py
+++ b/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py
@@ -610,6 +610,78 @@
 			pos_inv.delete()
 			pr.delete()
 
+	def test_delivered_serial_no_case(self):
+		from erpnext.accounts.doctype.pos_invoice_merge_log.test_pos_invoice_merge_log import (
+			init_user_and_profile,
+		)
+		from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
+		from erpnext.stock.doctype.serial_no.test_serial_no import get_serial_nos
+		from erpnext.stock.doctype.stock_entry.test_stock_entry import make_serialized_item
+
+		frappe.db.savepoint('before_test_delivered_serial_no_case')
+		try:
+			se = make_serialized_item()
+			serial_no = get_serial_nos(se.get("items")[0].serial_no)[0]
+
+			dn = create_delivery_note(
+					item_code="_Test Serialized Item With Series", serial_no=serial_no
+			)
+
+			delivery_document_no = frappe.db.get_value("Serial No", serial_no, "delivery_document_no")
+			self.assertEquals(delivery_document_no, dn.name)
+
+			init_user_and_profile()
+
+			pos_inv = create_pos_invoice(
+					item_code="_Test Serialized Item With Series",
+					serial_no=serial_no,
+					qty=1,
+					rate=100,
+					do_not_submit=True
+			)
+
+			self.assertRaises(frappe.ValidationError, pos_inv.submit)
+
+		finally:
+			frappe.db.rollback(save_point='before_test_delivered_serial_no_case')
+			frappe.set_user("Administrator")
+
+	def test_returned_serial_no_case(self):
+		from erpnext.accounts.doctype.pos_invoice_merge_log.test_pos_invoice_merge_log import (
+			init_user_and_profile,
+		)
+		from erpnext.stock.doctype.serial_no.serial_no import get_pos_reserved_serial_nos
+		from erpnext.stock.doctype.serial_no.test_serial_no import get_serial_nos
+		from erpnext.stock.doctype.stock_entry.test_stock_entry import make_serialized_item
+
+		frappe.db.savepoint('before_test_returned_serial_no_case')
+		try:
+			se = make_serialized_item()
+			serial_no = get_serial_nos(se.get("items")[0].serial_no)[0]
+
+			init_user_and_profile()
+
+			pos_inv = create_pos_invoice(
+					item_code="_Test Serialized Item With Series",
+					serial_no=serial_no,
+					qty=1,
+					rate=100,
+			)
+
+			pos_return = make_sales_return(pos_inv.name)
+			pos_return.flags.ignore_validate = True
+			pos_return.insert()
+			pos_return.submit()
+
+			pos_reserved_serial_nos = get_pos_reserved_serial_nos({
+				'item_code': '_Test Serialized Item With Series',
+				'warehouse': '_Test Warehouse - _TC'
+			})
+			self.assertTrue(serial_no not in pos_reserved_serial_nos)
+
+		finally:
+			frappe.db.rollback(save_point='before_test_returned_serial_no_case')
+			frappe.set_user("Administrator")
 
 def create_pos_invoice(**args):
 	args = frappe._dict(args)
diff --git a/erpnext/accounts/doctype/pos_profile/pos_profile.json b/erpnext/accounts/doctype/pos_profile/pos_profile.json
index 9c9f37b..11646a6 100644
--- a/erpnext/accounts/doctype/pos_profile/pos_profile.json
+++ b/erpnext/accounts/doctype/pos_profile/pos_profile.json
@@ -22,6 +22,7 @@
   "hide_images",
   "hide_unavailable_items",
   "auto_add_item_to_cart",
+  "validate_stock_on_save",
   "column_break_16",
   "update_stock",
   "ignore_pricing_rule",
@@ -351,6 +352,12 @@
   {
    "fieldname": "column_break_25",
    "fieldtype": "Column Break"
+  },
+  {
+   "default": "0",
+   "fieldname": "validate_stock_on_save",
+   "fieldtype": "Check",
+   "label": "Validate Stock on Save"
   }
  ],
  "icon": "icon-cog",
@@ -378,10 +385,11 @@
    "link_fieldname": "pos_profile"
   }
  ],
- "modified": "2021-10-14 14:17:00.469298",
+ "modified": "2022-03-21 13:29:28.480533",
  "modified_by": "Administrator",
  "module": "Accounts",
  "name": "POS Profile",
+ "naming_rule": "Set by user",
  "owner": "Administrator",
  "permissions": [
   {
@@ -404,5 +412,6 @@
   }
  ],
  "sort_field": "modified",
- "sort_order": "DESC"
+ "sort_order": "DESC",
+ "states": []
 }
\ No newline at end of file
diff --git a/erpnext/selling/page/point_of_sale/point_of_sale.py b/erpnext/selling/page/point_of_sale/point_of_sale.py
index 993c61d..67948d7 100644
--- a/erpnext/selling/page/point_of_sale/point_of_sale.py
+++ b/erpnext/selling/page/point_of_sale/point_of_sale.py
@@ -8,7 +8,7 @@
 from frappe.utils.nestedset import get_root_of
 
 from erpnext.accounts.doctype.pos_invoice.pos_invoice import get_stock_availability
-from erpnext.accounts.doctype.pos_profile.pos_profile import get_item_groups
+from erpnext.accounts.doctype.pos_profile.pos_profile import get_child_nodes, get_item_groups
 
 
 def search_by_term(search_term, warehouse, price_list):
@@ -275,3 +275,16 @@
 		contact_doc.set('phone_nos', [{ 'phone': value, 'is_primary_mobile_no': 1}])
 		frappe.db.set_value('Customer', customer, 'mobile_no', value)
 	contact_doc.save()
+
+@frappe.whitelist()
+def get_pos_profile_data(pos_profile):
+	pos_profile = frappe.get_doc('POS Profile', pos_profile)
+	pos_profile = pos_profile.as_dict()
+
+	_customer_groups_with_children = []
+	for row in pos_profile.customer_groups:
+		children = get_child_nodes('Customer Group', row.customer_group)
+		_customer_groups_with_children.extend(children)
+
+	pos_profile.customer_groups = _customer_groups_with_children
+	return pos_profile
\ No newline at end of file
diff --git a/erpnext/selling/page/point_of_sale/pos_controller.js b/erpnext/selling/page/point_of_sale/pos_controller.js
index ea8459f..6974bed 100644
--- a/erpnext/selling/page/point_of_sale/pos_controller.js
+++ b/erpnext/selling/page/point_of_sale/pos_controller.js
@@ -119,10 +119,15 @@
 			this.allow_negative_stock = flt(message.allow_negative_stock) || false;
 		});
 
-		frappe.db.get_doc("POS Profile", this.pos_profile).then((profile) => {
-			Object.assign(this.settings, profile);
-			this.settings.customer_groups = profile.customer_groups.map(group => group.customer_group);
-			this.make_app();
+		frappe.call({
+			method: "erpnext.selling.page.point_of_sale.point_of_sale.get_pos_profile_data",
+			args: { "pos_profile": this.pos_profile },
+			callback: (res) => {
+				const profile = res.message;
+				Object.assign(this.settings, profile);
+				this.settings.customer_groups = profile.customer_groups.map(group => group.name);
+				this.make_app();
+			}
 		});
 	}
 
@@ -555,7 +560,7 @@
 				if (this.item_details.$component.is(':visible'))
 					this.edit_item_details_of(item_row);
 
-				if (this.check_serial_batch_selection_needed(item_row))
+				if (this.check_serial_batch_selection_needed(item_row) && !this.item_details.$component.is(':visible'))
 					this.edit_item_details_of(item_row);
 			}
 
@@ -704,7 +709,7 @@
 		frappe.dom.freeze();
 		const { doctype, name, current_item } = this.item_details;
 
-		frappe.model.set_value(doctype, name, 'qty', 0)
+		return frappe.model.set_value(doctype, name, 'qty', 0)
 			.then(() => {
 				frappe.model.clear_doc(doctype, name);
 				this.update_cart_html(current_item, true);
@@ -715,7 +720,14 @@
 	}
 
 	async save_and_checkout() {
-		this.frm.is_dirty() && await this.frm.save();
-		this.payment.checkout();
+		if (this.frm.is_dirty()) {
+			// only move to payment section if save is successful
+			frappe.route_hooks.after_save = () => this.payment.checkout();
+			return this.frm.save(
+				null, null, null, () => this.cart.toggle_checkout_btn(true) // show checkout button on error
+			);
+		} else {
+			this.payment.checkout();
+		}
 	}
 };
diff --git a/erpnext/selling/page/point_of_sale/pos_item_details.js b/erpnext/selling/page/point_of_sale/pos_item_details.js
index fb69b63..b75ffb2 100644
--- a/erpnext/selling/page/point_of_sale/pos_item_details.js
+++ b/erpnext/selling/page/point_of_sale/pos_item_details.js
@@ -60,12 +60,18 @@
 		return item && item.name == this.current_item.name;
 	}
 
-	toggle_item_details_section(item) {
+	async toggle_item_details_section(item) {
 		const current_item_changed = !this.compare_with_current_item(item);
 
 		// if item is null or highlighted cart item is clicked twice
 		const hide_item_details = !Boolean(item) || !current_item_changed;
 
+		if ((!hide_item_details && current_item_changed) || hide_item_details) {
+			// if item details is being closed OR if item details is opened but item is changed
+			// in both cases, if the current item is a serialized item, then validate and remove the item
+			await this.validate_serial_batch_item();
+		}
+
 		this.events.toggle_item_selector(!hide_item_details);
 		this.toggle_component(!hide_item_details);
 
@@ -83,7 +89,6 @@
 			this.render_form(item);
 			this.events.highlight_cart_item(item);
 		} else {
-			this.validate_serial_batch_item();
 			this.current_item = {};
 		}
 	}
@@ -103,11 +108,11 @@
 			(serialized && batched && (no_batch_selected || no_serial_selected))) {
 
 			frappe.show_alert({
-				message: __("Item will be removed since no serial / batch no selected."),
+				message: __("Item is removed since no serial / batch no selected."),
 				indicator: 'orange'
 			});
 			frappe.utils.play_sound("cancel");
-			this.events.remove_item_from_cart();
+			return this.events.remove_item_from_cart();
 		}
 	}
 
diff --git a/erpnext/selling/page/point_of_sale/pos_payment.js b/erpnext/selling/page/point_of_sale/pos_payment.js
index 1e9f6d7..b4ece46 100644
--- a/erpnext/selling/page/point_of_sale/pos_payment.js
+++ b/erpnext/selling/page/point_of_sale/pos_payment.js
@@ -170,20 +170,24 @@
     });
 
 		frappe.ui.form.on('POS Invoice', 'coupon_code', (frm) => {
-			if (!frm.doc.ignore_pricing_rule && frm.doc.coupon_code) {
-				frappe.run_serially([
-					() => frm.doc.ignore_pricing_rule=1,
-					() => frm.trigger('ignore_pricing_rule'),
-					() => frm.doc.ignore_pricing_rule=0,
-					() => frm.trigger('apply_pricing_rule'),
-					() => frm.save(),
-					() => this.update_totals_section(frm.doc)
-				]);
-			} else if (frm.doc.ignore_pricing_rule && frm.doc.coupon_code) {
-				frappe.show_alert({
-					message: __("Ignore Pricing Rule is enabled. Cannot apply coupon code."),
-					indicator: "orange"
-				});
+			if (frm.doc.coupon_code && !frm.applying_pos_coupon_code) {
+				if (!frm.doc.ignore_pricing_rule) {
+					frm.applying_pos_coupon_code = true;
+					frappe.run_serially([
+						() => frm.doc.ignore_pricing_rule=1,
+						() => frm.trigger('ignore_pricing_rule'),
+						() => frm.doc.ignore_pricing_rule=0,
+						() => frm.trigger('apply_pricing_rule'),
+						() => frm.save(),
+						() => this.update_totals_section(frm.doc),
+						() => (frm.applying_pos_coupon_code = false)
+					]);
+				} else if (frm.doc.ignore_pricing_rule) {
+					frappe.show_alert({
+						message: __("Ignore Pricing Rule is enabled. Cannot apply coupon code."),
+						indicator: "orange"
+					});
+				}
 			}
 		});
 
diff --git a/erpnext/stock/doctype/serial_no/serial_no.py b/erpnext/stock/doctype/serial_no/serial_no.py
index 4c06012..66c7707 100644
--- a/erpnext/stock/doctype/serial_no/serial_no.py
+++ b/erpnext/stock/doctype/serial_no/serial_no.py
@@ -611,25 +611,65 @@
 
 	return sorted([d.get('name') for d in serial_numbers])
 
+def get_delivered_serial_nos(serial_nos):
+	'''
+	Returns serial numbers that delivered from the list of serial numbers
+	'''
+	from frappe.query_builder.functions import Coalesce
+
+	SerialNo = frappe.qb.DocType("Serial No")
+	serial_nos = get_serial_nos(serial_nos)
+	query = (
+		frappe.qb
+			.from_(SerialNo)
+			.select(SerialNo.name)
+			.where(
+				(SerialNo.name.isin(serial_nos))
+				& (Coalesce(SerialNo.delivery_document_type, "") != "")
+			)
+	)
+
+	result = query.run()
+	if result and len(result) > 0:
+		delivered_serial_nos = [row[0] for row in result]
+		return delivered_serial_nos
+
 @frappe.whitelist()
 def get_pos_reserved_serial_nos(filters):
 	if isinstance(filters, str):
 		filters = json.loads(filters)
 
-	pos_transacted_sr_nos = frappe.db.sql("""select item.serial_no as serial_no
-		from `tabPOS Invoice` p, `tabPOS Invoice Item` item
-		where p.name = item.parent
-		and p.consolidated_invoice is NULL
-		and p.docstatus = 1
-		and item.docstatus = 1
-		and item.item_code = %(item_code)s
-		and item.warehouse = %(warehouse)s
-		and item.serial_no is NOT NULL and item.serial_no != ''
-		""", filters, as_dict=1)
+	POSInvoice = frappe.qb.DocType("POS Invoice")
+	POSInvoiceItem = frappe.qb.DocType("POS Invoice Item")
+	query = frappe.qb.from_(
+		POSInvoice
+	).from_(
+		POSInvoiceItem
+	).select(
+		POSInvoice.is_return,
+		POSInvoiceItem.serial_no
+	).where(
+		(POSInvoice.name == POSInvoiceItem.parent)
+		& (POSInvoice.docstatus == 1)
+		& (POSInvoiceItem.docstatus == 1)
+		& (POSInvoiceItem.item_code == filters.get('item_code'))
+		& (POSInvoiceItem.warehouse == filters.get('warehouse'))
+		& (POSInvoiceItem.serial_no.isnotnull())
+		& (POSInvoiceItem.serial_no != '')
+	)
+
+	pos_transacted_sr_nos = query.run(as_dict=True)
 
 	reserved_sr_nos = []
+	returned_sr_nos = []
 	for d in pos_transacted_sr_nos:
-		reserved_sr_nos += get_serial_nos(d.serial_no)
+		if d.is_return == 0:
+			reserved_sr_nos += get_serial_nos(d.serial_no)
+		elif d.is_return == 1:
+			returned_sr_nos += get_serial_nos(d.serial_no)
+
+	for sr_no in returned_sr_nos:
+		reserved_sr_nos.remove(sr_no)
 
 	return reserved_sr_nos