From 50d56db0c2ffcbb5ac466e0b8f1b7330dc78c9b3 Mon Sep 17 00:00:00 2001 From: Dany Robert Date: Mon, 29 Jan 2024 09:32:44 +0530 Subject: [PATCH] fix: specify precision for net_amount (#39481) * fix: specify precision for net_amount * fix: correct existing test to account for precision * fix: rounding issue in test cases * fix: optional grand total manipulation * fix: use `grand_total_diff` for manipulation * fix: patch to set default for grand total manipulation * fix: wrong rounding assertion for USD * fix: undefined this.frm error * chore: linters * fix: `net_amount` percision and method rename * fix: missing frm reference * chore: minor cleanups and depr message * refactor: remove optional adjusting of grand total --- .../accounts_settings/accounts_settings.json | 2 +- .../doctype/pos_invoice/test_pos_invoice.py | 2 +- .../test_pos_invoice_merge_log.py | 2 +- .../sales_invoice/test_sales_invoice.py | 65 +++++++++++-------- erpnext/controllers/taxes_and_totals.py | 25 ++++--- erpnext/public/js/controllers/accounts.js | 9 +-- .../public/js/controllers/taxes_and_totals.js | 28 +++++--- 7 files changed, 82 insertions(+), 51 deletions(-) diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json index fd052d0476..b10924ff38 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json @@ -462,7 +462,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2023-11-20 09:37:47.650347", + "modified": "2024-01-22 12:10:10.151819", "modified_by": "Administrator", "module": "Accounts", "name": "Accounts Settings", diff --git a/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py b/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py index 200b82a447..8fab290e07 100644 --- a/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py +++ b/erpnext/accounts/doctype/pos_invoice/test_pos_invoice.py @@ -93,7 +93,7 @@ class TestPOSInvoice(unittest.TestCase): inv.save() - self.assertEqual(inv.net_total, 4298.25) + self.assertEqual(inv.net_total, 4298.24) self.assertEqual(inv.grand_total, 4900.00) def test_tax_calculation_with_multiple_items(self): 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 6af8a0015b..0ad9131445 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 @@ -351,7 +351,7 @@ class TestPOSInvoiceMergeLog(unittest.TestCase): inv.load_from_db() consolidated_invoice = frappe.get_doc("Sales Invoice", inv.consolidated_invoice) self.assertEqual(consolidated_invoice.status, "Return") - self.assertEqual(consolidated_invoice.rounding_adjustment, -0.001) + self.assertEqual(consolidated_invoice.rounding_adjustment, -0.002) finally: frappe.set_user("Administrator") diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index cb4b2fc914..672fec2fee 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -323,7 +323,8 @@ class TestSalesInvoice(FrappeTestCase): si.insert() # with inclusive tax - self.assertEqual(si.items[0].net_amount, 3947.368421052631) + self.assertEqual(si.items[0].net_amount, 3947.37) + self.assertEqual(si.net_total, si.base_net_total) self.assertEqual(si.net_total, 3947.37) self.assertEqual(si.grand_total, 5000) @@ -667,7 +668,7 @@ class TestSalesInvoice(FrappeTestCase): 62.5, 625.0, 50, - 499.97600115194473, + 499.98, ], "_Test Item Home Desktop 200": [ 190.66, @@ -678,7 +679,7 @@ class TestSalesInvoice(FrappeTestCase): 190.66, 953.3, 150, - 749.9968530500239, + 750, ], } @@ -691,20 +692,21 @@ class TestSalesInvoice(FrappeTestCase): self.assertEqual(d.get(k), expected_values[d.item_code][i]) # check net total - self.assertEqual(si.net_total, 1249.97) + self.assertEqual(si.base_net_total, si.net_total) + self.assertEqual(si.net_total, 1249.98) self.assertEqual(si.total, 1578.3) # check tax calculation expected_values = { "keys": ["tax_amount", "total"], - "_Test Account Excise Duty - _TC": [140, 1389.97], - "_Test Account Education Cess - _TC": [2.8, 1392.77], - "_Test Account S&H Education Cess - _TC": [1.4, 1394.17], - "_Test Account CST - _TC": [27.88, 1422.05], - "_Test Account VAT - _TC": [156.25, 1578.30], - "_Test Account Customs Duty - _TC": [125, 1703.30], - "_Test Account Shipping Charges - _TC": [100, 1803.30], - "_Test Account Discount - _TC": [-180.33, 1622.97], + "_Test Account Excise Duty - _TC": [140, 1389.98], + "_Test Account Education Cess - _TC": [2.8, 1392.78], + "_Test Account S&H Education Cess - _TC": [1.4, 1394.18], + "_Test Account CST - _TC": [27.88, 1422.06], + "_Test Account VAT - _TC": [156.25, 1578.31], + "_Test Account Customs Duty - _TC": [125, 1703.31], + "_Test Account Shipping Charges - _TC": [100, 1803.31], + "_Test Account Discount - _TC": [-180.33, 1622.98], } for d in si.get("taxes"): @@ -740,7 +742,7 @@ class TestSalesInvoice(FrappeTestCase): "base_rate": 2500, "base_amount": 25000, "net_rate": 40, - "net_amount": 399.9808009215558, + "net_amount": 399.98, "base_net_rate": 2000, "base_net_amount": 19999, }, @@ -754,7 +756,7 @@ class TestSalesInvoice(FrappeTestCase): "base_rate": 7500, "base_amount": 37500, "net_rate": 118.01, - "net_amount": 590.0531205155963, + "net_amount": 590.05, "base_net_rate": 5900.5, "base_net_amount": 29502.5, }, @@ -792,8 +794,13 @@ class TestSalesInvoice(FrappeTestCase): self.assertEqual(si.base_grand_total, 60795) self.assertEqual(si.grand_total, 1215.90) - self.assertEqual(si.rounding_adjustment, 0.01) - self.assertEqual(si.base_rounding_adjustment, 0.50) + # no rounding adjustment as the Smallest Currency Fraction Value of USD is 0.01 + if frappe.db.get_value("Currency", "USD", "smallest_currency_fraction_value") < 0.01: + self.assertEqual(si.rounding_adjustment, 0.10) + self.assertEqual(si.base_rounding_adjustment, 5.0) + else: + self.assertEqual(si.rounding_adjustment, 0.0) + self.assertEqual(si.base_rounding_adjustment, 0.0) def test_outstanding(self): w = self.make() @@ -2082,7 +2089,7 @@ class TestSalesInvoice(FrappeTestCase): def test_rounding_adjustment_2(self): si = create_sales_invoice(rate=400, do_not_save=True) - for rate in [400, 600, 100]: + for rate in [400.25, 600.30, 100.65]: si.append( "items", { @@ -2108,17 +2115,18 @@ class TestSalesInvoice(FrappeTestCase): ) si.save() si.submit() - self.assertEqual(si.net_total, 1271.19) - self.assertEqual(si.grand_total, 1500) - self.assertEqual(si.total_taxes_and_charges, 228.82) - self.assertEqual(si.rounding_adjustment, -0.01) + self.assertEqual(si.net_total, si.base_net_total) + self.assertEqual(si.net_total, 1272.20) + self.assertEqual(si.grand_total, 1501.20) + self.assertEqual(si.total_taxes_and_charges, 229) + self.assertEqual(si.rounding_adjustment, -0.20) expected_values = [ - ["_Test Account Service Tax - _TC", 0.0, 114.41], - ["_Test Account VAT - _TC", 0.0, 114.41], - [si.debit_to, 1500, 0.0], - ["Round Off - _TC", 0.01, 0.01], - ["Sales - _TC", 0.0, 1271.18], + ["_Test Account Service Tax - _TC", 0.0, 114.50], + ["_Test Account VAT - _TC", 0.0, 114.50], + [si.debit_to, 1501, 0.0], + ["Round Off - _TC", 0.20, 0.0], + ["Sales - _TC", 0.0, 1272.20], ] gl_entries = frappe.db.sql( @@ -2176,7 +2184,8 @@ class TestSalesInvoice(FrappeTestCase): si.save() si.submit() - self.assertEqual(si.net_total, 4007.16) + self.assertEqual(si.net_total, si.base_net_total) + self.assertEqual(si.net_total, 4007.15) self.assertEqual(si.grand_total, 4488.02) self.assertEqual(si.total_taxes_and_charges, 480.86) self.assertEqual(si.rounding_adjustment, -0.02) @@ -2188,7 +2197,7 @@ class TestSalesInvoice(FrappeTestCase): ["_Test Account Service Tax - _TC", 0.0, 240.43], ["_Test Account VAT - _TC", 0.0, 240.43], ["Sales - _TC", 0.0, 4007.15], - ["Round Off - _TC", 0.02, 0.01], + ["Round Off - _TC", 0.01, 0.0], ] ) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index f9f68a119b..8cb1a0e861 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -8,6 +8,7 @@ import frappe from frappe import _, scrub from frappe.model.document import Document from frappe.utils import cint, flt, round_based_on_smallest_currency_fraction +from frappe.utils.deprecations import deprecated import erpnext from erpnext.accounts.doctype.journal_entry.journal_entry import get_exchange_rate @@ -74,7 +75,7 @@ class calculate_taxes_and_totals(object): self.calculate_net_total() self.calculate_tax_withholding_net_total() self.calculate_taxes() - self.manipulate_grand_total_for_inclusive_tax() + self.adjust_grand_total_for_inclusive_tax() self.calculate_totals() self._cleanup() self.calculate_total_net_weight() @@ -279,7 +280,7 @@ class calculate_taxes_and_totals(object): ): amount = flt(item.amount) - total_inclusive_tax_amount_per_qty - item.net_amount = flt(amount / (1 + cumulated_tax_fraction)) + item.net_amount = flt(amount / (1 + cumulated_tax_fraction), item.precision("net_amount")) item.net_rate = flt(item.net_amount / item.qty, item.precision("net_rate")) item.discount_percentage = flt(item.discount_percentage, item.precision("discount_percentage")) @@ -516,7 +517,12 @@ class calculate_taxes_and_totals(object): tax.base_tax_amount = round(tax.base_tax_amount, 0) tax.base_tax_amount_after_discount_amount = round(tax.base_tax_amount_after_discount_amount, 0) + @deprecated def manipulate_grand_total_for_inclusive_tax(self): + # for backward compatablility - if in case used by an external application + return self.adjust_grand_total_for_inclusive_tax() + + def adjust_grand_total_for_inclusive_tax(self): # if fully inclusive taxes and diff if self.doc.get("taxes") and any(cint(t.included_in_print_rate) for t in self.doc.get("taxes")): last_tax = self.doc.get("taxes")[-1] @@ -538,17 +544,21 @@ class calculate_taxes_and_totals(object): diff = flt(diff, self.doc.precision("rounding_adjustment")) if diff and abs(diff) <= (5.0 / 10 ** last_tax.precision("tax_amount")): - self.doc.rounding_adjustment = diff + self.doc.grand_total_diff = diff + else: + self.doc.grand_total_diff = 0 def calculate_totals(self): if self.doc.get("taxes"): - self.doc.grand_total = flt(self.doc.get("taxes")[-1].total) + flt(self.doc.rounding_adjustment) + self.doc.grand_total = flt(self.doc.get("taxes")[-1].total) + flt( + self.doc.get("grand_total_diff") + ) else: self.doc.grand_total = flt(self.doc.net_total) if self.doc.get("taxes"): self.doc.total_taxes_and_charges = flt( - self.doc.grand_total - self.doc.net_total - flt(self.doc.rounding_adjustment), + self.doc.grand_total - self.doc.net_total - flt(self.doc.get("grand_total_diff")), self.doc.precision("total_taxes_and_charges"), ) else: @@ -613,8 +623,8 @@ class calculate_taxes_and_totals(object): self.doc.grand_total, self.doc.currency, self.doc.precision("rounded_total") ) - # if print_in_rate is set, we would have already calculated rounding adjustment - self.doc.rounding_adjustment += flt( + # rounding adjustment should always be the difference vetween grand and rounded total + self.doc.rounding_adjustment = flt( self.doc.rounded_total - self.doc.grand_total, self.doc.precision("rounding_adjustment") ) @@ -832,7 +842,6 @@ class calculate_taxes_and_totals(object): self.calculate_paid_amount() def calculate_paid_amount(self): - paid_amount = base_paid_amount = 0.0 if self.doc.is_pos: diff --git a/erpnext/public/js/controllers/accounts.js b/erpnext/public/js/controllers/accounts.js index 7879173cd1..f0d8cbb99c 100644 --- a/erpnext/public/js/controllers/accounts.js +++ b/erpnext/public/js/controllers/accounts.js @@ -160,7 +160,7 @@ erpnext.accounts.taxes = { let tax = frappe.get_doc(cdt, cdn); try { me.validate_taxes_and_charges(cdt, cdn); - me.validate_inclusive_tax(tax); + me.validate_inclusive_tax(tax, frm); } catch(e) { tax.included_in_print_rate = 0; refresh_field("included_in_print_rate", tax.name, tax.parentfield); @@ -170,7 +170,8 @@ erpnext.accounts.taxes = { }); }, - validate_inclusive_tax: function(tax) { + validate_inclusive_tax: function(tax, frm) { + this.frm = this.frm || frm; let actual_type_error = function() { var msg = __("Actual type tax cannot be included in Item rate in row {0}", [tax.idx]) frappe.throw(msg); @@ -186,12 +187,12 @@ erpnext.accounts.taxes = { if(tax.charge_type == "Actual") { // inclusive tax cannot be of type Actual actual_type_error(); - } else if(tax.charge_type == "On Previous Row Amount" && + } else if (tax.charge_type == "On Previous Row Amount" && this.frm && !cint(this.frm.doc["taxes"][tax.row_id - 1].included_in_print_rate) ) { // referred row should also be an inclusive tax on_previous_row_error(tax.row_id); - } else if(tax.charge_type == "On Previous Row Total") { + } else if (tax.charge_type == "On Previous Row Total" && this.frm) { var taxes_not_included = $.map(this.frm.doc["taxes"].slice(0, tax.row_id), function(t) { return cint(t.included_in_print_rate) ? null : t; }); if(taxes_not_included.length > 0) { diff --git a/erpnext/public/js/controllers/taxes_and_totals.js b/erpnext/public/js/controllers/taxes_and_totals.js index d24c4e6075..bd5a136a15 100644 --- a/erpnext/public/js/controllers/taxes_and_totals.js +++ b/erpnext/public/js/controllers/taxes_and_totals.js @@ -103,7 +103,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { this.determine_exclusive_rate(); this.calculate_net_total(); this.calculate_taxes(); - this.manipulate_grand_total_for_inclusive_tax(); + this.adjust_grand_total_for_inclusive_tax(); this.calculate_totals(); this._cleanup(); } @@ -185,7 +185,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { if (!this.discount_amount_applied) { erpnext.accounts.taxes.validate_taxes_and_charges(tax.doctype, tax.name); - erpnext.accounts.taxes.validate_inclusive_tax(tax); + erpnext.accounts.taxes.validate_inclusive_tax(tax, this.frm); } frappe.model.round_floats_in(tax); }); @@ -248,7 +248,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { if(!me.discount_amount_applied && item.qty && (total_inclusive_tax_amount_per_qty || cumulated_tax_fraction)) { var amount = flt(item.amount) - total_inclusive_tax_amount_per_qty; - item.net_amount = flt(amount / (1 + cumulated_tax_fraction)); + item.net_amount = flt(amount / (1 + cumulated_tax_fraction), precision("net_amount", item)); item.net_rate = item.qty ? flt(item.net_amount / item.qty, precision("net_rate", item)) : 0; me.set_in_company_currency(item, ["net_rate", "net_amount"]); @@ -303,6 +303,8 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { me.frm.doc.net_total += item.net_amount; me.frm.doc.base_net_total += item.base_net_amount; }); + + frappe.model.round_floats_in(this.frm.doc, ["total", "base_total", "net_total", "base_net_total"]); } calculate_shipping_charges() { @@ -521,8 +523,17 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { } } + /** + * @deprecated Use adjust_grand_total_for_inclusive_tax instead. + */ manipulate_grand_total_for_inclusive_tax() { + // for backward compatablility - if in case used by an external application + this.adjust_grand_total_for_inclusive_tax() + } + + adjust_grand_total_for_inclusive_tax() { var me = this; + // if fully inclusive taxes and diff if (this.frm.doc["taxes"] && this.frm.doc["taxes"].length) { var any_inclusive_tax = false; @@ -548,7 +559,9 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { diff = flt(diff, precision("rounding_adjustment")); if ( diff && Math.abs(diff) <= (5.0 / Math.pow(10, precision("tax_amount", last_tax))) ) { - me.frm.doc.rounding_adjustment = diff; + me.frm.doc.grand_total_diff = diff; + } else { + me.frm.doc.grand_total_diff = 0; } } } @@ -559,7 +572,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { var me = this; var tax_count = this.frm.doc["taxes"] ? this.frm.doc["taxes"].length : 0; this.frm.doc.grand_total = flt(tax_count - ? this.frm.doc["taxes"][tax_count - 1].total + flt(this.frm.doc.rounding_adjustment) + ? this.frm.doc["taxes"][tax_count - 1].total + flt(this.frm.doc.grand_total_diff) : this.frm.doc.net_total); if(in_list(["Quotation", "Sales Order", "Delivery Note", "Sales Invoice", "POS Invoice"], this.frm.doc.doctype)) { @@ -619,7 +632,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { if(frappe.meta.get_docfield(this.frm.doc.doctype, "rounded_total", this.frm.doc.name)) { this.frm.doc.rounded_total = round_based_on_smallest_currency_fraction(this.frm.doc.grand_total, this.frm.doc.currency, precision("rounded_total")); - this.frm.doc.rounding_adjustment += flt(this.frm.doc.rounded_total - this.frm.doc.grand_total, + this.frm.doc.rounding_adjustment = flt(this.frm.doc.rounded_total - this.frm.doc.grand_total, precision("rounding_adjustment")); this.set_in_company_currency(this.frm.doc, ["rounding_adjustment", "rounded_total"]); @@ -687,8 +700,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { if (total_for_discount_amount) { $.each(this.frm._items || [], function(i, item) { distributed_amount = flt(me.frm.doc.discount_amount) * item.net_amount / total_for_discount_amount; - item.net_amount = flt(item.net_amount - distributed_amount, - precision("base_amount", item)); + item.net_amount = flt(item.net_amount - distributed_amount, precision("net_amount", item)); net_total += item.net_amount; // discount amount rounding loss adjustment if no taxes