diff --git a/erpnext/accounts/doctype/pos_invoice_merge_log/pos_invoice_merge_log.py b/erpnext/accounts/doctype/pos_invoice_merge_log/pos_invoice_merge_log.py index 05ebbdd45b4..4909ba74793 100644 --- a/erpnext/accounts/doctype/pos_invoice_merge_log/pos_invoice_merge_log.py +++ b/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 diff --git a/erpnext/accounts/doctype/pos_invoice_merge_log/test_pos_invoice_merge_log.py b/erpnext/accounts/doctype/pos_invoice_merge_log/test_pos_invoice_merge_log.py index be7206fc9bd..0e67a582bf5 100644 --- a/erpnext/accounts/doctype/pos_invoice_merge_log/test_pos_invoice_merge_log.py +++ b/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`")