From 0452d7de20a8eddc1403d20b5f6cfba12eb63e82 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 8 Feb 2022 11:26:23 +0530 Subject: [PATCH 1/6] fix(pos): incorrect grand_total in case of inclusive taxes on item --- .../pos_invoice_merge_log.py | 26 +++- .../test_pos_invoice_merge_log.py | 115 ++++++++++++++++++ erpnext/controllers/taxes_and_totals.py | 3 + 3 files changed, 140 insertions(+), 4 deletions(-) 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 0720d9b2e9..f372dd604c 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 @@ -84,12 +84,21 @@ class POSInvoiceMergeLog(Document): sales_invoice.set_posting_time = 1 sales_invoice.posting_date = getdate(self.posting_date) sales_invoice.save() + self.write_off_fractional_amount(sales_invoice, data) sales_invoice.submit() self.consolidated_invoice = sales_invoice.name return sales_invoice.name + def write_off_fractional_amount(self, invoice, data): + pos_invoice_grand_total = sum(d.grand_total for d in data) + + if abs(pos_invoice_grand_total - invoice.grand_total) < 1: + + invoice.write_off_amount += -1 * (pos_invoice_grand_total - invoice.grand_total) + invoice.save() + def process_merging_into_credit_note(self, data): credit_note = self.get_new_sales_invoice() credit_note.is_return = 1 @@ -102,6 +111,7 @@ class POSInvoiceMergeLog(Document): # TODO: return could be against multiple sales invoice which could also have been consolidated? # credit_note.return_against = self.consolidated_invoice credit_note.save() + self.write_off_fractional_amount(credit_note, data) credit_note.submit() self.consolidated_credit_note = credit_note.name @@ -135,9 +145,15 @@ class POSInvoiceMergeLog(Document): i.uom == item.uom and i.net_rate == item.net_rate and i.warehouse == item.warehouse): found = True i.qty = i.qty + item.qty + i.amount = i.amount + item.net_amount + i.net_amount = i.amount + i.base_amount = i.base_amount + item.base_net_amount + i.base_net_amount = i.base_amount if not found: item.rate = item.net_rate + item.amount = item.net_amount + item.base_amount = item.base_net_amount item.price_list_rate = 0 si_item = map_child_doc(item, invoice, {"doctype": "Sales Invoice Item"}) items.append(si_item) @@ -169,10 +185,12 @@ class POSInvoiceMergeLog(Document): found = True if not found: payments.append(payment) - rounding_adjustment += doc.rounding_adjustment - rounded_total += doc.rounded_total - base_rounding_adjustment += doc.base_rounding_adjustment - base_rounded_total += doc.base_rounded_total + + if doc.rounding_adjustment or doc.base_rounding_adjustment: + rounding_adjustment += doc.rounding_adjustment + rounded_total += doc.rounded_total + base_rounding_adjustment += doc.base_rounding_adjustment + base_rounded_total += doc.base_rounded_total if loyalty_points_sum: 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 3555da83a4..928d26676d 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 @@ -150,3 +150,118 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): frappe.set_user("Administrator") frappe.db.sql("delete from `tabPOS Profile`") frappe.db.sql("delete from `tabPOS Invoice`") + + + def test_consolidation_round_off_error_1(self): + ''' + Test case for bug: + Round off error in consolidated invoice creation if POS Invoice has inclusive tax + ''' + frappe.db.sql("delete from `tabPOS Invoice`") + + try: + init_user_and_profile() + + inv = create_pos_invoice(qty=3, rate=10000, do_not_save=True) + inv.append("taxes", { + "account_head": "_Test Account VAT - _TC", + "charge_type": "On Net Total", + "cost_center": "_Test Cost Center - _TC", + "description": "VAT", + "doctype": "Sales Taxes and Charges", + "rate": 7.5, + "included_in_print_rate": 1 + }) + inv.append('payments', { + 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': 30000 + }) + inv.insert() + inv.submit() + + inv2 = create_pos_invoice(qty=3, rate=10000, do_not_save=True) + inv2.append("taxes", { + "account_head": "_Test Account VAT - _TC", + "charge_type": "On Net Total", + "cost_center": "_Test Cost Center - _TC", + "description": "VAT", + "doctype": "Sales Taxes and Charges", + "rate": 7.5, + "included_in_print_rate": 1 + }) + inv2.append('payments', { + 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': 30000 + }) + inv2.insert() + inv2.submit() + + consolidate_pos_invoices() + + inv.load_from_db() + consolidated_invoice = frappe.get_doc('Sales Invoice', inv.consolidated_invoice) + self.assertEqual(consolidated_invoice.outstanding_amount, 0) + self.assertEqual(consolidated_invoice.status, 'Paid') + + finally: + frappe.set_user("Administrator") + frappe.db.sql("delete from `tabPOS Profile`") + frappe.db.sql("delete from `tabPOS Invoice`") + + def test_consolidation_round_off_error_2(self): + ''' + Test the same case as above but with an Unpaid POS Invoice + ''' + frappe.db.sql("delete from `tabPOS Invoice`") + + try: + init_user_and_profile() + + inv = create_pos_invoice(qty=6, rate=10000, do_not_save=True) + inv.append("taxes", { + "account_head": "_Test Account VAT - _TC", + "charge_type": "On Net Total", + "cost_center": "_Test Cost Center - _TC", + "description": "VAT", + "doctype": "Sales Taxes and Charges", + "rate": 7.5, + "included_in_print_rate": 1 + }) + inv.append('payments', { + 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': 60000 + }) + inv.insert() + inv.submit() + + inv2 = create_pos_invoice(qty=6, rate=10000, do_not_save=True) + inv2.append("taxes", { + "account_head": "_Test Account VAT - _TC", + "charge_type": "On Net Total", + "cost_center": "_Test Cost Center - _TC", + "description": "VAT", + "doctype": "Sales Taxes and Charges", + "rate": 7.5, + "included_in_print_rate": 1 + }) + inv2.append('payments', { + 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': 60000 + }) + inv2.insert() + inv2.submit() + + inv3 = create_pos_invoice(qty=3, rate=600, do_not_save=True) + inv3.append('payments', { + 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': 1000 + }) + inv3.insert() + inv3.submit() + + consolidate_pos_invoices() + + inv.load_from_db() + consolidated_invoice = frappe.get_doc('Sales Invoice', inv.consolidated_invoice) + self.assertEqual(consolidated_invoice.outstanding_amount, 800) + self.assertEqual(consolidated_invoice.status, 'Unpaid') + + finally: + frappe.set_user("Administrator") + frappe.db.sql("delete from `tabPOS Profile`") + frappe.db.sql("delete from `tabPOS Invoice`") diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index 075e3e38fa..5d1856cfa9 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -106,6 +106,9 @@ class calculate_taxes_and_totals(object): self.doc.conversion_rate = flt(self.doc.conversion_rate) def calculate_item_values(self): + if self.doc.get('is_consolidated'): + return + if not self.discount_amount_applied: for item in self.doc.get("items"): self.doc.round_floats_in(item) From afc5c26d1c7ba2973f8e74d57029e78db550946b Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 8 Feb 2022 16:04:08 +0530 Subject: [PATCH 2/6] fix(test): ignore stock validation --- .../pos_invoice_merge_log/test_pos_invoice_merge_log.py | 4 ++++ 1 file changed, 4 insertions(+) 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 928d26676d..fd1aaab264 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 @@ -158,6 +158,7 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): Round off error in consolidated invoice creation if POS Invoice has inclusive tax ''' frappe.db.sql("delete from `tabPOS Invoice`") + frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 1) try: init_user_and_profile() @@ -205,12 +206,14 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): frappe.set_user("Administrator") frappe.db.sql("delete from `tabPOS Profile`") frappe.db.sql("delete from `tabPOS Invoice`") + frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 0) def test_consolidation_round_off_error_2(self): ''' Test the same case as above but with an Unpaid POS Invoice ''' frappe.db.sql("delete from `tabPOS Invoice`") + frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 1) try: init_user_and_profile() @@ -265,3 +268,4 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): frappe.set_user("Administrator") frappe.db.sql("delete from `tabPOS Profile`") frappe.db.sql("delete from `tabPOS Invoice`") + frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 0) From c2b83a02837e8ab9c2e23596f22b7f75e420003f Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 8 Feb 2022 17:07:51 +0530 Subject: [PATCH 3/6] fix(test): case if write off is calculated as negative amount --- .../pos_invoice_merge_log/pos_invoice_merge_log.py | 1 - .../test_pos_invoice_merge_log.py | 11 ++++++----- erpnext/controllers/taxes_and_totals.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) 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 d98dec8b92..ddca68a57b 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 @@ -95,7 +95,6 @@ class POSInvoiceMergeLog(Document): pos_invoice_grand_total = sum(d.grand_total for d in data) if abs(pos_invoice_grand_total - invoice.grand_total) < 1: - invoice.write_off_amount += -1 * (pos_invoice_grand_total - invoice.grand_total) invoice.save() 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 fd1aaab264..fc14161456 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 @@ -154,10 +154,10 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): def test_consolidation_round_off_error_1(self): ''' - Test case for bug: - Round off error in consolidated invoice creation if POS Invoice has inclusive tax + Test round off error in consolidated invoice creation if POS Invoice has inclusive tax ''' frappe.db.sql("delete from `tabPOS Invoice`") + allow_negative_stock = frappe.db.get_value('Stock Settings', None, 'allow_negative_stock') frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 1) try: @@ -206,13 +206,14 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): frappe.set_user("Administrator") frappe.db.sql("delete from `tabPOS Profile`") frappe.db.sql("delete from `tabPOS Invoice`") - frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 0) + frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', allow_negative_stock) def test_consolidation_round_off_error_2(self): ''' Test the same case as above but with an Unpaid POS Invoice ''' frappe.db.sql("delete from `tabPOS Invoice`") + allow_negative_stock = frappe.db.get_value('Stock Settings', None, 'allow_negative_stock') frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 1) try: @@ -262,10 +263,10 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): inv.load_from_db() consolidated_invoice = frappe.get_doc('Sales Invoice', inv.consolidated_invoice) self.assertEqual(consolidated_invoice.outstanding_amount, 800) - self.assertEqual(consolidated_invoice.status, 'Unpaid') + self.assertNotEqual(consolidated_invoice.status, 'Paid') finally: frappe.set_user("Administrator") frappe.db.sql("delete from `tabPOS Profile`") frappe.db.sql("delete from `tabPOS Invoice`") - frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 0) + frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', allow_negative_stock) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index 5d1856cfa9..de1099ee28 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -650,12 +650,12 @@ class calculate_taxes_and_totals(object): def calculate_change_amount(self): self.doc.change_amount = 0.0 self.doc.base_change_amount = 0.0 + grand_total = self.doc.rounded_total or self.doc.grand_total + base_grand_total = self.doc.base_rounded_total or self.doc.base_grand_total if self.doc.doctype == "Sales Invoice" \ - and self.doc.paid_amount > self.doc.grand_total and not self.doc.is_return \ + and self.doc.paid_amount > grand_total and not self.doc.is_return \ and any(d.type == "Cash" for d in self.doc.payments): - grand_total = self.doc.rounded_total or self.doc.grand_total - base_grand_total = self.doc.base_rounded_total or self.doc.base_grand_total self.doc.change_amount = flt(self.doc.paid_amount - grand_total + self.doc.write_off_amount, self.doc.precision("change_amount")) From 75256863c6e3ed917d3ff00a9435da9fa7115cbb Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Wed, 9 Feb 2022 10:05:06 +0530 Subject: [PATCH 4/6] fix(test): do not enable negative stock --- .../test_pos_invoice_merge_log.py | 22 ++++++++++++++----- erpnext/controllers/taxes_and_totals.py | 2 +- 2 files changed, 17 insertions(+), 7 deletions(-) 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 fc14161456..5930aa097f 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 @@ -12,6 +12,7 @@ from erpnext.accounts.doctype.pos_invoice.test_pos_invoice import create_pos_inv from erpnext.accounts.doctype.pos_invoice_merge_log.pos_invoice_merge_log import ( consolidate_pos_invoices, ) +from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry class TestPOSInvoiceMergeLog(unittest.TestCase): @@ -156,11 +157,17 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): ''' Test round off error in consolidated invoice creation if POS Invoice has inclusive tax ''' + frappe.db.sql("delete from `tabPOS Invoice`") - allow_negative_stock = frappe.db.get_value('Stock Settings', None, 'allow_negative_stock') - frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 1) try: + make_stock_entry( + to_warehouse="_Test Warehouse - _TC", + item_code="_Test Item", + rate=8000, + qty=10, + ) + init_user_and_profile() inv = create_pos_invoice(qty=3, rate=10000, do_not_save=True) @@ -206,17 +213,21 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): frappe.set_user("Administrator") frappe.db.sql("delete from `tabPOS Profile`") frappe.db.sql("delete from `tabPOS Invoice`") - frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', allow_negative_stock) def test_consolidation_round_off_error_2(self): ''' Test the same case as above but with an Unpaid POS Invoice ''' frappe.db.sql("delete from `tabPOS Invoice`") - allow_negative_stock = frappe.db.get_value('Stock Settings', None, 'allow_negative_stock') - frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', 1) try: + make_stock_entry( + to_warehouse="_Test Warehouse - _TC", + item_code="_Test Item", + rate=8000, + qty=10, + ) + init_user_and_profile() inv = create_pos_invoice(qty=6, rate=10000, do_not_save=True) @@ -269,4 +280,3 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): frappe.set_user("Administrator") frappe.db.sql("delete from `tabPOS Profile`") frappe.db.sql("delete from `tabPOS Invoice`") - frappe.db.set_value('Stock Settings', None, 'allow_negative_stock', allow_negative_stock) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index de1099ee28..2776628227 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -654,7 +654,7 @@ class calculate_taxes_and_totals(object): base_grand_total = self.doc.base_rounded_total or self.doc.base_grand_total if self.doc.doctype == "Sales Invoice" \ - and self.doc.paid_amount > grand_total and not self.doc.is_return \ + and self.doc.paid_amount > grand_total and not self.doc.is_return \ and any(d.type == "Cash" for d in self.doc.payments): self.doc.change_amount = flt(self.doc.paid_amount - grand_total + From 4bb557dbd84b109b83de12b2e77a60d953c292ea Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Wed, 9 Feb 2022 12:06:59 +0530 Subject: [PATCH 5/6] fix: flaky point of sale test --- erpnext/tests/test_point_of_sale.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/erpnext/tests/test_point_of_sale.py b/erpnext/tests/test_point_of_sale.py index df2dc8b99a..3299c8885f 100644 --- a/erpnext/tests/test_point_of_sale.py +++ b/erpnext/tests/test_point_of_sale.py @@ -1,15 +1,25 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # MIT License. See license.txt +import unittest + +import frappe from erpnext.accounts.doctype.pos_profile.test_pos_profile import make_pos_profile from erpnext.selling.page.point_of_sale.point_of_sale import get_items from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry -from erpnext.tests.utils import ERPNextTestCase -class TestPointOfSale(ERPNextTestCase): +class TestPointOfSale(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + frappe.db.savepoint('before_test_point_of_sale') + + @classmethod + def tearDownClass(cls) -> None: + frappe.db.rollback(save_point='before_test_point_of_sale') + def test_item_search(self): """ Test Stock and Service Item Search. From 0ebd32dc630daf03dc77f81a93944a1919f0c016 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Wed, 9 Feb 2022 16:08:28 +0530 Subject: [PATCH 6/6] fix: cancelling of consolidated sales invoice that doesn't have closing entry --- erpnext/accounts/doctype/sales_invoice/sales_invoice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 754ca81424..b894f90c7e 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -285,7 +285,7 @@ class SalesInvoice(SellingController): filters={ invoice_or_credit_note: self.name }, pluck="pos_closing_entry" ) - if pos_closing_entry: + if pos_closing_entry and pos_closing_entry[0]: msg = _("To cancel a {} you need to cancel the POS Closing Entry {}.").format( frappe.bold("Consolidated Sales Invoice"), get_link_to_form("POS Closing Entry", pos_closing_entry[0])