fix: improved rounding adjustment when applying discount (backport #46720)

* fix: improved rounding adjustment when applying discount (#46720)

* fix: rounding adjustment in apply_discount_amount taxes_and_totals

* refactor: minor changes

* fix: set the rounding difference while calculating tax total in the last tax row and add test case

* fix: failing test case

* fix: made changes in get_total_for_discount_amount in taxes_and_totals

* fix: failing test cases

* fix: changes as per review

* refactor: remove unnecessary use of flt

* refactor: improve logic

* refactor: minor change

* refactor: minor changes

* fix: add a test case for applying discount with previous row total in taxes

* fix: failing test case

* refactor: flatter code, remove `flt` usage for accuracy

---------

Co-authored-by: Vishakh Desai <78500008+vishakhdesai@users.noreply.github.com>
Co-authored-by: Sagar Vora <sagar@resilient.tech>

erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py

@@ -1984,6 +1984,78 @@ class TestPurchaseInvoice(FrappeTestCase, StockTestMixin):
 
 		self.assertRaises(frappe.ValidationError, dr_note.save)
 
+	def test_apply_discount_on_grand_total(self):
+		"""
+		To test if after applying discount on grand total,
+		the grand total is calculated correctly without any rounding errors
+		"""
+		invoice = make_purchase_invoice(qty=2, rate=100, do_not_save=True, do_not_submit=True)
+		invoice.append(
+			"items",
+			{
+				"item_code": "_Test Item",
+				"qty": 1,
+				"rate": 21.39,
+			},
+		)
+		invoice.append(
+			"taxes",
+			{
+				"charge_type": "On Net Total",
+				"account_head": "_Test Account VAT - _TC",
+				"description": "VAT",
+				"rate": 15.5,
+			},
+		)
+
+		# the grand total here will be 255.71
+		invoice.disable_rounded_total = 1
+		# apply discount on grand total to adjust the grand total to 255
+		invoice.discount_amount = 0.71
+		invoice.save()
+
+		# check if grand total is 496 and not something like 254.99 due to rounding errors
+		self.assertEqual(invoice.grand_total, 255)
+
+	def test_apply_discount_on_grand_total_with_previous_row_total_tax(self):
+		"""
+		To test if after applying discount on grand total,
+		where the tax is calculated on previous row total, the grand total is calculated correctly
+		"""
+
+		invoice = make_purchase_invoice(qty=2, rate=100, do_not_save=True, do_not_submit=True)
+		invoice.extend(
+			"taxes",
+			[
+				{
+					"charge_type": "Actual",
+					"account_head": "_Test Account VAT - _TC",
+					"description": "VAT",
+					"tax_amount": 100,
+				},
+				{
+					"charge_type": "On Previous Row Amount",
+					"account_head": "_Test Account VAT - _TC",
+					"description": "VAT",
+					"row_id": 1,
+					"rate": 10,
+				},
+				{
+					"charge_type": "On Previous Row Total",
+					"account_head": "_Test Account VAT - _TC",
+					"description": "VAT",
+					"row_id": 1,
+					"rate": 10,
+				},
+			],
+		)
+
+		# the total here will be 340, so applying 40 discount
+		invoice.discount_amount = 40
+		invoice.save()
+
+		self.assertEqual(invoice.grand_total, 300)
+
 
 def check_gl_entries(
 	doc,

erpnext/controllers/taxes_and_totals.py

@@ -619,7 +619,7 @@ class calculate_taxes_and_totals:
 
 		if self.doc.meta.get_field("rounded_total"):
 			if self.doc.is_rounded_total_disabled():
-				self.doc.rounded_total = self.doc.base_rounded_total = 0
+				self.doc.rounded_total = self.doc.base_rounded_total = self.doc.rounding_adjustment = 0
 				return
 
 			self.doc.rounded_total = round_based_on_smallest_currency_fraction(
@@ -663,33 +663,29 @@ class calculate_taxes_and_totals:
 				return
 
 			total_for_discount_amount = self.get_total_for_discount_amount()
-			taxes = self.doc.get("taxes")
 			net_total = 0
+			expected_net_total = 0
 
 			if total_for_discount_amount:
 				# calculate item amount after Discount Amount
-				for i, item in enumerate(self._items):
+				for item in self._items:
 					distributed_amount = (
 						flt(self.doc.discount_amount) * item.net_amount / total_for_discount_amount
 					)
 
-					item.net_amount = flt(item.net_amount - distributed_amount, item.precision("net_amount"))
+					adjusted_net_amount = item.net_amount - distributed_amount
+					expected_net_total += adjusted_net_amount
+					item.net_amount = flt(adjusted_net_amount, item.precision("net_amount"))
 					net_total += item.net_amount
 
-					# discount amount rounding loss adjustment if no taxes
-					if (
-						self.doc.apply_discount_on == "Net Total"
-						or not taxes
-						or total_for_discount_amount == self.doc.net_total
-					) and i == len(self._items) - 1:
-						discount_amount_loss = flt(
-							self.doc.net_total - net_total - self.doc.discount_amount,
-							self.doc.precision("net_total"),
-						)
-
+					# discount amount rounding adjustment
+					if rounding_difference := flt(
+						expected_net_total - net_total, self.doc.precision("net_total")
+					):
 						item.net_amount = flt(
-							item.net_amount + discount_amount_loss, item.precision("net_amount")
+							item.net_amount + rounding_difference, item.precision("net_amount")
 						)
+						net_total += rounding_difference
 
 					item.net_rate = (
 						flt(item.net_amount / item.qty, item.precision("net_rate")) if item.qty else 0
@@ -705,20 +701,44 @@ class calculate_taxes_and_totals:
 	def get_total_for_discount_amount(self):
 		if self.doc.apply_discount_on == "Net Total":
 			return self.doc.net_total
-		else:
-			actual_taxes_dict = {}
 
-			for tax in self.doc.get("taxes"):
-				if tax.charge_type in ["Actual", "On Item Quantity"]:
-					tax_amount = self.get_tax_amount_if_for_valuation_or_deduction(tax.tax_amount, tax)
-					actual_taxes_dict.setdefault(tax.idx, tax_amount)
-				elif tax.row_id in actual_taxes_dict:
-					actual_tax_amount = flt(actual_taxes_dict.get(tax.row_id, 0)) * flt(tax.rate) / 100
-					actual_taxes_dict.setdefault(tax.idx, actual_tax_amount)
+		total_actual_tax = 0
+		actual_taxes_dict = {}
 
-			return flt(
-				self.doc.grand_total - sum(actual_taxes_dict.values()), self.doc.precision("grand_total")
+		def update_actual_tax_dict(tax, tax_amount):
+			nonlocal total_actual_tax
+
+			if tax.get("add_deduct_tax") == "Deduct":
+				tax_amount *= -1
+
+			if tax.get("category") != "Valuation":
+				total_actual_tax += tax_amount
+
+			actual_taxes_dict[int(tax.idx)] = {
+				"tax_amount": tax_amount,
+				"cumulative_tax_amount": total_actual_tax,
+			}
+
+		for tax in self.doc.get("taxes"):
+			if tax.charge_type in ["Actual", "On Item Quantity"]:
+				update_actual_tax_dict(tax, tax.tax_amount)
+				continue
+
+			if not tax.row_id:
+				continue
+
+			base_row = actual_taxes_dict.get(int(tax.row_id))
+			if not base_row:
+				continue
+
+			base_tax_amount = (
+				base_row["tax_amount"]
+				if tax.charge_type == "On Previous Row Amount"
+				else base_row["cumulative_tax_amount"]
 			)
+			update_actual_tax_dict(tax, base_tax_amount * tax.rate / 100)
+
+		return self.doc.grand_total - total_actual_tax
 
 	def calculate_total_advance(self):
 		if not self.doc.docstatus.is_cancelled():

erpnext/public/js/controllers/taxes_and_totals.js

@@ -336,7 +336,6 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments {
 
 	calculate_taxes() {
 		var me = this;
-		this.frm.doc.rounding_adjustment = 0;
 		var actual_tax_dict = {};
 
 		// maintain actual tax rate based on idx
@@ -589,7 +588,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments {
 		}
 
 		this.frm.doc.total_taxes_and_charges = flt(this.frm.doc.grand_total - this.frm.doc.net_total
-			- flt(this.frm.doc.rounding_adjustment), precision("total_taxes_and_charges"));
+			- flt(this.frm.doc.grand_total_diff), precision("total_taxes_and_charges"));
 
 		this.set_in_company_currency(this.frm.doc, ["total_taxes_and_charges", "rounding_adjustment"]);
 
@@ -611,6 +610,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments {
 		if (cint(disable_rounded_total)) {
 			this.frm.doc.rounded_total = 0;
 			this.frm.doc.base_rounded_total = 0;
+			this.frm.doc.rounding_adjustment = 0;
 			return;
 		}
 
@@ -679,22 +679,26 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments {
 				return;
 			}
 
-			var total_for_discount_amount = this.get_total_for_discount_amount();
-			var net_total = 0;
+			const total_for_discount_amount = this.get_total_for_discount_amount();
+			let net_total = 0;
+			let expected_net_total = 0;
+
 			// calculate item amount after Discount Amount
 			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("net_amount", item));
+
+					const adjusted_net_amount = item.net_amount - distributed_amount;
+					expected_net_total += adjusted_net_amount
+					item.net_amount = flt(adjusted_net_amount, precision("net_amount", item));
 					net_total += item.net_amount;
 
-					// discount amount rounding loss adjustment if no taxes
-					if ((!(me.frm.doc.taxes || []).length || total_for_discount_amount==me.frm.doc.net_total || (me.frm.doc.apply_discount_on == "Net Total"))
-							&& i == (me.frm._items || []).length - 1) {
-						var discount_amount_loss = flt(me.frm.doc.net_total - net_total
-							- me.frm.doc.discount_amount, precision("net_total"));
-						item.net_amount = flt(item.net_amount + discount_amount_loss,
-							precision("net_amount", item));
+					// discount amount rounding adjustment
+					// assignment to rounding_difference is intentional
+					const rounding_difference = flt(expected_net_total - net_total, precision("net_total"));
+					if (rounding_difference) {
+						item.net_amount = flt(item.net_amount + rounding_difference, precision("net_amount", item));
+						net_total += rounding_difference;
 					}
 					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"]);
@@ -707,29 +711,38 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments {
 	}
 
 	get_total_for_discount_amount() {
-		if(this.frm.doc.apply_discount_on == "Net Total") {
+		if(this.frm.doc.apply_discount_on == "Net Total")
 			return this.frm.doc.net_total;
-		} else {
-			var total_actual_tax = 0.0;
-			var actual_taxes_dict = {};
 
-			$.each(this.frm.doc["taxes"] || [], function(i, tax) {
-				if (["Actual", "On Item Quantity"].includes(tax.charge_type)) {
-					var tax_amount = (tax.category == "Valuation") ? 0.0 : tax.tax_amount;
-					tax_amount *= (tax.add_deduct_tax == "Deduct") ? -1.0 : 1.0;
-					actual_taxes_dict[tax.idx] = tax_amount;
-				} else if (actual_taxes_dict[tax.row_id] !== null) {
-					var actual_tax_amount = flt(actual_taxes_dict[tax.row_id]) * flt(tax.rate) / 100;
-					actual_taxes_dict[tax.idx] = actual_tax_amount;
-				}
-			});
+		let total_actual_tax = 0.0;
+		let actual_taxes_dict = {};
 
-			$.each(actual_taxes_dict, function(key, value) {
-				if (value) total_actual_tax += value;
-			});
+		function update_actual_taxes_dict(tax, tax_amount) {
+			if (tax.add_deduct_tax == "Deduct") tax_amount *= -1;
+			if (tax.category != "Valuation") total_actual_tax += tax_amount;
 
-			return flt(this.frm.doc.grand_total - total_actual_tax, precision("grand_total"));
+			actual_taxes_dict[tax.idx] = {
+				tax_amount: tax_amount,
+				cumulative_total: total_actual_tax
+			};
 		}
+
+		$.each(this.frm.doc["taxes"] || [], function(i, tax) {
+			if (["Actual", "On Item Quantity"].includes(tax.charge_type)) {
+				update_actual_taxes_dict(tax, tax.tax_amount);
+				return;
+			}
+
+			const base_row = actual_taxes_dict[tax.row_id];
+			if (!base_row) return;
+
+			// if charge type is 'On Previous Row Amount', calculate tax on previous row amount
+			// else (On Previous Row Total) calculate tax on cumulative total
+			const base_tax_amount = tax.charge_type == "On Previous Row Amount" ? base_row["tax_amount"]: base_row["cumulative_total"];
+			update_actual_taxes_dict(tax, base_tax_amount * tax.rate / 100);
+		});
+
+		return this.frm.doc.grand_total - total_actual_tax;
 	}
 
 	calculate_total_advance(update_paid_amount) {