fix(pos): quote the reversed row's rate on a consolidated credit note (backport #59320) (#59322)

Co-authored-by: Diptanil Saha <diptanil@frappe.io>

erpnext/accounts/doctype/pos_invoice_merge_log/pos_invoice_merge_log.py

@@ -217,6 +217,8 @@ class POSInvoiceMergeLog(Document):
 
 		loyalty_amount_sum, loyalty_points_sum, idx = 0, 0, 1
 
+		reversed_rows = get_reversed_rows([doc.return_against for doc in data if doc.is_return])
+
 		for doc in data:
 			map_doc(doc, invoice, table_map={"doctype": invoice.doctype})
 
@@ -239,9 +241,13 @@ class POSInvoiceMergeLog(Document):
 				si_item.pos_invoice = doc.name
 				si_item.pos_invoice_item = item.name
 				if doc.is_return:
-					si_item.sales_invoice_item = get_sales_invoice_item(
-						doc.return_against, item.pos_invoice_item
-					)
+					reversed_row = reversed_rows.get(item.pos_invoice_item) or frappe._dict()
+					si_item.sales_invoice_item = reversed_row.get("name")
+					# quote the rate of the row being reversed: rounding an invoice-level discount
+					# can leave a return's net rate a minor unit above the sale's, and
+					# validate_returned_items refuses a return priced above its original
+					if si_item.sales_invoice_item:
+						si_item.rate = reversed_row.rate
 				if item.serial_and_batch_bundle:
 					si_item.serial_and_batch_bundle = item.serial_and_batch_bundle
 				items.append(si_item)
@@ -439,6 +445,29 @@ def update_item_wise_tax_detail(consolidate_tax_row, tax_row):
 	consolidate_tax_row.item_wise_tax_detail = json.dumps(consolidated_tax_detail, separators=(",", ":"))
 
 
+def get_reversed_rows(return_against):
+	"""Rows of the consolidated sales these returns reverse, keyed by the POS invoice row."""
+	if not return_against:
+		return {}
+
+	sales_invoice = DocType("Sales Invoice")
+	sales_invoice_item = DocType("Sales Invoice Item")
+
+	rows = (
+		frappe.qb.from_(sales_invoice)
+		.from_(sales_invoice_item)
+		.select(sales_invoice_item.name, sales_invoice_item.rate, sales_invoice_item.pos_invoice_item)
+		.where(
+			(sales_invoice.name == sales_invoice_item.parent)
+			& (sales_invoice.is_return == 0)
+			& (sales_invoice_item.pos_invoice.isin(return_against))
+			& (sales_invoice.docstatus == 1)
+		)
+	).run(as_dict=True)
+
+	return {row.pos_invoice_item: row for row in rows}
+
+
 def get_all_unconsolidated_invoices():
 	filters = {
 		"consolidated_invoice": ["in", ["", None]],
@@ -683,27 +712,3 @@ def get_error_message(message) -> str:
 		return message["message"]
 	except Exception:
 		return str(message)
-
-
-def get_sales_invoice_item(return_against_pos_invoice, pos_invoice_item):
-	try:
-		SalesInvoice = DocType("Sales Invoice")
-		SalesInvoiceItem = DocType("Sales Invoice Item")
-
-		query = (
-			frappe.qb.from_(SalesInvoice)
-			.from_(SalesInvoiceItem)
-			.select(SalesInvoiceItem.name)
-			.where(
-				(SalesInvoice.name == SalesInvoiceItem.parent)
-				& (SalesInvoice.is_return == 0)
-				& (SalesInvoiceItem.pos_invoice == return_against_pos_invoice)
-				& (SalesInvoiceItem.pos_invoice_item == pos_invoice_item)
-				& (SalesInvoice.docstatus == 1)
-			)
-		)
-
-		result = query.run(as_dict=True)
-		return result[0].name if result else None
-	except Exception:
-		return None

erpnext/accounts/doctype/pos_invoice_merge_log/test_pos_invoice_merge_log.py

@@ -3,9 +3,11 @@
 
 import json
 import unittest
+from contextlib import contextmanager
 
 import frappe
 from frappe.tests.utils import change_settings
+from frappe.utils import flt
 
 from erpnext.accounts.doctype.pos_closing_entry.test_pos_closing_entry import init_user_and_profile
 from erpnext.accounts.doctype.pos_invoice.pos_invoice import make_sales_return
@@ -19,6 +21,67 @@ from erpnext.stock.doctype.serial_and_batch_bundle.test_serial_and_batch_bundle
 from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
 
 
+@contextmanager
+def rounding_method(method):
+	"""System Settings is also cached on frappe.local, so that copy has to go as well."""
+	previous = frappe.db.get_single_value("System Settings", "rounding_method")
+	try:
+		frappe.db.set_single_value("System Settings", "rounding_method", method)
+		frappe.local.system_settings = None
+		yield
+	finally:
+		frappe.db.set_single_value("System Settings", "rounding_method", previous)
+		frappe.local.system_settings = None
+
+
+def sell_over_the_counter(lines, discount_percentage=0):
+	item_code, qty, rate = lines[0]
+	sale = create_pos_invoice(item_code=item_code, qty=qty, rate=rate, do_not_save=True)
+	for item_code, qty, rate in lines[1:]:
+		sale.append(
+			"items",
+			{
+				"item_code": item_code,
+				"qty": qty,
+				"rate": rate,
+				"price_list_rate": rate,
+				"warehouse": "_Test Warehouse - _TC",
+				"income_account": "Sales - _TC",
+				"cost_center": "_Test Cost Center - _TC",
+			},
+		)
+
+	if discount_percentage:
+		sale.apply_discount_on = "Net Total"
+		sale.additional_discount_percentage = discount_percentage
+
+	sale.run_method("calculate_taxes_and_totals")
+	payable = sale.rounded_total or sale.grand_total
+	sale.append("payments", {"mode_of_payment": "Cash", "account": "Cash - _TC", "amount": payable})
+	sale.paid_amount = sale.base_paid_amount = payable
+	sale.insert()
+	sale.submit()
+	return sale
+
+
+def refund_over_the_counter(sale, qty=None):
+	"""Hand back every line of `sale`, `qty` of each when fewer units come back."""
+	note = make_sales_return(sale.name)
+	if qty is not None:
+		for item in note.items:
+			item.qty = qty
+
+	note.run_method("calculate_taxes_and_totals")
+	refundable = note.rounded_total or note.grand_total
+	note.payments[0].amount = refundable
+	for spare in note.payments[1:]:
+		spare.amount = 0
+	note.paid_amount = note.base_paid_amount = refundable
+	note.insert()
+	note.submit()
+	return note
+
+
 class TestPOSInvoiceMergeLog(unittest.TestCase):
 	def test_consolidated_invoice_creation(self):
 		frappe.db.sql("delete from `tabPOS Invoice`")
@@ -510,3 +573,92 @@ class TestPOSInvoiceMergeLog(unittest.TestCase):
 			frappe.set_user("Administrator")
 			frappe.db.sql("delete from `tabPOS Profile`")
 			frappe.db.sql("delete from `tabPOS Invoice`")
+
+	@change_settings("Selling Settings", {"allow_multiple_items": 1})
+	def test_consolidating_returns_priced_off_a_rounded_invoice_discount(self):
+		"""A return works out its own share of an invoice-level discount, so rounding can leave
+		it a minor unit above the sale's, and validate_returned_items then refuses it.
+
+		Every shape that reaches a consolidated credit note goes through one consolidation:
+		a split landing on a half minor unit, the same item on two rows so the rows can only
+		be paired through sales_invoice_item, fewer units coming back than went out, and — as
+		a control — a sale with no invoice-level discount to split at all.
+		"""
+		frappe.db.sql("delete from `tabPOS Invoice`")
+
+		try:
+			for item_code in ("_Test Item", "_Test Item 2"):
+				make_stock_entry(to_warehouse="_Test Warehouse - _TC", item_code=item_code, rate=100, qty=40)
+			init_user_and_profile()
+
+			with rounding_method("Banker's Rounding (legacy)"):
+				tied = sell_over_the_counter(
+					[("_Test Item", 1, 42.86), ("_Test Item 2", 1, 57.14)], discount_percentage=25
+				)
+				repeated = sell_over_the_counter(
+					[("_Test Item", 1, 42.86), ("_Test Item", 1, 57.14)], discount_percentage=25
+				)
+				oversold = sell_over_the_counter(
+					[("_Test Item", 3, 42.86), ("_Test Item 2", 3, 57.14)], discount_percentage=25
+				)
+				undiscounted = sell_over_the_counter([("_Test Item", 1, 42.86), ("_Test Item 2", 1, 57.14)])
+
+				# the sale and the return really do round the split apart
+				self.assertEqual(
+					{item.item_code: item.net_rate for item in tied.items},
+					{"_Test Item": 32.15, "_Test Item 2": 42.85},
+				)
+				returns = [
+					refund_over_the_counter(tied),
+					refund_over_the_counter(repeated),
+					refund_over_the_counter(oversold, qty=-1),
+					refund_over_the_counter(undiscounted),
+				]
+				self.assertEqual(
+					{item.item_code: item.net_rate for item in returns[0].items},
+					{"_Test Item": 32.14, "_Test Item 2": 42.86},
+				)
+
+				consolidate_pos_invoices()
+
+			for pos_invoice in [tied, repeated, oversold, undiscounted, *returns]:
+				pos_invoice.load_from_db()
+				self.assertTrue(
+					frappe.db.exists("Sales Invoice", pos_invoice.consolidated_invoice),
+					f"{pos_invoice.name} was not consolidated",
+				)
+				self.assertEqual(
+					frappe.db.get_value(
+						"Sales Invoice", pos_invoice.consolidated_invoice, "outstanding_amount"
+					),
+					0,
+				)
+
+			for note in returns:
+				# no returned row may be priced above the row it reverses
+				for row in frappe.get_all(
+					"Sales Invoice Item",
+					filters={"parent": note.consolidated_invoice},
+					fields=["item_code", "rate", "sales_invoice_item"],
+				):
+					self.assertTrue(row.sales_invoice_item, f"{row.item_code} lost its link to the sale")
+					sold_rate = frappe.db.get_value("Sales Invoice Item", row.sales_invoice_item, "rate")
+					self.assertLessEqual(row.rate, sold_rate)
+
+			# returns for one customer land on a single credit note, which still adds up to
+			# everything handed back over the counter
+			refunded = {}
+			for note in returns:
+				refunded[note.consolidated_invoice] = refunded.get(note.consolidated_invoice, 0) + flt(
+					note.grand_total
+				)
+			for consolidated_name, handed_back in refunded.items():
+				self.assertEqual(
+					flt(frappe.db.get_value("Sales Invoice", consolidated_name, "grand_total"), 2),
+					flt(handed_back, 2),
+				)
+
+		finally:
+			frappe.set_user("Administrator")
+			frappe.db.sql("delete from `tabPOS Profile`")
+			frappe.db.sql("delete from `tabPOS Invoice`")