From 4cc8c14757bfab134f59986d9d89f8f3c55cd8be Mon Sep 17 00:00:00 2001 From: Sagar Vora <16315650+sagarvora@users.noreply.github.com> Date: Fri, 17 Oct 2025 15:31:21 +0530 Subject: [PATCH 1/6] fix: ensure that additional discount amount is not mapped repeatedly --- erpnext/controllers/accounts_controller.py | 79 ++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 5d4630d7d5..74fa7692f2 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -40,6 +40,8 @@ from erpnext.accounts.doctype.pricing_rule.utils import ( ) from erpnext.accounts.general_ledger import get_round_off_account_and_cost_center from erpnext.accounts.party import ( + PURCHASE_TRANSACTION_TYPES, + SALES_TRANSACTION_TYPES, get_party_account, get_party_account_currency, get_party_gle_currency, @@ -3143,6 +3145,83 @@ class AccountsController(TransactionBase): x["transaction_currency"] = self.currency x["transaction_exchange_rate"] = self.get("conversion_rate") or 1 + def after_mapping(self, source_doc): + self.set_discount_amount_after_mapping(source_doc) + + def set_discount_amount_after_mapping(self, source_doc): + """ + Ensures that Additional Discount Amount is not copied repeatedly + for multiple mappings of a single source transaction. + """ + + # source and target doctypes should both be buying / selling + for transaction_types in (PURCHASE_TRANSACTION_TYPES, SALES_TRANSACTION_TYPES): + if self.doctype in transaction_types and source_doc.doctype in transaction_types: + break + + else: + return + + # ensure both doctypes have discount_amount field + if not self.meta.get_field("discount_amount") or not source_doc.meta.get_field("discount_amount"): + return + + # ensure discount_amount is set in source doc + if not source_doc.discount_amount: + return + + # ensure additional_discount_percentage is not set in the source doc + if source_doc.get("additional_discount_percentage"): + return + + item_doctype = self.meta.get_field("items").options + item_meta = frappe.get_meta(item_doctype) + + reference_fieldname = next( + ( + row.fieldname + for row in item_meta.fields + if row.fieldtype == "Link" + and row.options == source_doc.doctype + and not row.get("is_custom_field") + ), + None, + ) + + if not reference_fieldname: + return + + doctype_table = frappe.qb.DocType(self.doctype) + item_table = frappe.qb.DocType(item_doctype) + discount_already_applied = ( + frappe.qb.from_(doctype_table) + .where(doctype_table.docstatus == 1) + .where(doctype_table.discount_amount != 0) + .where( + doctype_table.name.isin( + frappe.qb.from_(item_table) + .select(item_table.parent) + .where(item_table[reference_fieldname] == source_doc.name) + .distinct() + ) + ) + .select(Sum(doctype_table.discount_amount)) + ).run() + + if not discount_already_applied: + return + + discount_already_applied = flt(discount_already_applied[0][0], self.precision("discount_amount")) + if (source_doc.discount_amount * (discount_already_applied - source_doc.discount_amount)) >= 0: + # full discount already applied or exceeded + self.discount_amount = 0 + else: + self.discount_amount = flt( + self.discount_amount - discount_already_applied, self.precision("discount_amount") + ) + + self.calculate_taxes_and_totals() + @frappe.whitelist() def get_tax_rate(account_head): -- GitLab From def6d7f5757603079002859bc75ea5dd268d1e41 Mon Sep 17 00:00:00 2001 From: Sagar Vora <16315650+sagarvora@users.noreply.github.com> Date: Fri, 17 Oct 2025 16:12:30 +0530 Subject: [PATCH 2/6] fix: validate that discount amount cannot exceed total before discount --- erpnext/controllers/taxes_and_totals.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index d3c73387f9..5e6b689748 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -7,7 +7,7 @@ import json 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 import cint, flt, fmt_money, round_based_on_smallest_currency_fraction import erpnext from erpnext.accounts.doctype.journal_entry.journal_entry import get_exchange_rate @@ -724,6 +724,22 @@ class calculate_taxes_and_totals: self.doc.precision("discount_amount"), ) + discount_amount = self.doc.discount_amount or 0 + grand_total = self.doc.grand_total + + # validate that discount amount cannot exceed the total before discount + if grand_total * (discount_amount - grand_total) > 0: + frappe.throw( + _( + "Additional Discount Amount ({discount_amount}) cannot exceed " + "the total before such discount ({total_before_discount})" + ).format( + discount_amount=self.doc.get_formatted("discount_amount"), + total_before_discount=self.doc.get_formatted("grand_total"), + ), + title=_("Invalid Discount Amount"), + ) + def apply_discount_amount(self): if self.doc.discount_amount: if not self.doc.apply_discount_on: -- GitLab From 05119000ee5099ccba38466abb243ee0b694acda Mon Sep 17 00:00:00 2001 From: Sagar Vora <16315650+sagarvora@users.noreply.github.com> Date: Fri, 17 Oct 2025 17:32:16 +0530 Subject: [PATCH 3/6] test: some tests to ensure correct discount mapping --- .../tests/test_accounts_controller.py | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 706268a662..2c86ab620e 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -2284,3 +2284,141 @@ class TestAccountsController(IntegrationTestCase): self.assertRaises(frappe.ValidationError, si.save) si.contact_person = customer_contact.name si.save() + + def test_discount_amount_not_mapped_repeatedly_for_sales_transactions(self): + """ + Test that additional discount amount is not copied repeatedly + when creating multiple delivery notes from a single sales order with discount_amount set + """ + from erpnext.selling.doctype.sales_order.sales_order import make_delivery_note + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + + # Create a sales order with discount amount + so = make_sales_order(qty=10, rate=100, do_not_submit=True) + so.apply_discount_on = "Net Total" + so.discount_amount = 100 + so.save() + so.submit() + + # Create first delivery note from sales order (partial qty) + dn1 = make_delivery_note(so.name) + dn1.items[0].qty = 5 + dn1.save() + dn1.submit() + + # First delivery note should have full discount amount + self.assertEqual(dn1.discount_amount, 100) + self.assertEqual(dn1.grand_total, 400) + + # Create second delivery note from the same sales order (remaining qty) + dn2 = make_delivery_note(so.name) + dn2.items[0].qty = 5 + dn2.save() + dn2.submit() + + # Second delivery note should have discount_amount set to 0 + # because discount was already fully applied in first delivery note + self.assertEqual(dn2.discount_amount, 0) + self.assertEqual(dn2.grand_total, 500) + + def test_discount_amount_not_mapped_repeatedly_for_purchase_transactions(self): + """ + Test that additional discount amount is not copied repeatedly + when creating multiple purchase receipts from a single purchase order with discount_amount set + """ + from erpnext.buying.doctype.purchase_order.purchase_order import make_purchase_receipt + from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order + + # Create a purchase order with discount amount + po = create_purchase_order(qty=10, rate=100, do_not_submit=True) + po.apply_discount_on = "Net Total" + po.discount_amount = 100 + po.save() + po.submit() + + # Create first purchase receipt from purchase order (partial qty) + pr1 = make_purchase_receipt(po.name) + pr1.items[0].qty = 5 + pr1.save() + pr1.submit() + + # First purchase receipt should have full discount amount + self.assertEqual(pr1.discount_amount, 100) + self.assertEqual(pr1.grand_total, 400) + + # Create second purchase receipt from the same purchase order (remaining qty) + pr2 = make_purchase_receipt(po.name) + pr2.items[0].qty = 5 + pr2.save() + pr2.submit() + + # Second purchase receipt should have discount_amount set to 0 + # because discount was already fully applied in first purchase receipt + self.assertEqual(pr2.discount_amount, 0) + self.assertEqual(pr2.grand_total, 500) + + def test_discount_amount_partial_application_in_mapped_transactions(self): + """ + Test that discount amount is partially applied when some discount + has already been used in previous mapped transactions + """ + from erpnext.selling.doctype.sales_order.sales_order import make_sales_invoice + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + + # Create a sales order with discount amount + so = make_sales_order(qty=10, rate=100, do_not_submit=True) + so.apply_discount_on = "Net Total" + so.discount_amount = 200 + so.save() + so.submit() + + self.assertEqual(so.discount_amount, 200) + self.assertEqual(so.grand_total, 800) + + # Create first invoice with partial discount (manually set lower discount) + si1 = make_sales_invoice(so.name) + si1.items[0].qty = 5 + si1.discount_amount = 50 # Partial discount application + si1.save() + si1.submit() + + self.assertEqual(si1.discount_amount, 50) + self.assertEqual(si1.grand_total, 450) + + # Create second invoice from the same sales order + si2 = make_sales_invoice(so.name) + si2.items[0].qty = 5 + si2.save() + si2.submit() + + # Second invoice should have remaining discount (200 - 50 = 150) + self.assertEqual(si2.discount_amount, 150) + self.assertEqual(si2.grand_total, 350) + + def test_discount_amount_not_mapped_when_percentage_is_set(self): + """ + Test that discount amount is not adjusted when additional_discount_percentage + is set in the source document (as it will be recalculated based on percentage) + """ + from erpnext.selling.doctype.sales_order.sales_order import make_delivery_note + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + + # Create a sales order with discount percentage instead of amount + so = make_sales_order(qty=10, rate=100, do_not_submit=True) + so.apply_discount_on = "Net Total" + so.additional_discount_percentage = 10 # 10% discount + so.save() + so.submit() + + self.assertEqual(so.discount_amount, 100) # 10% of 1000 + self.assertEqual(so.grand_total, 900) + + # Create delivery note from sales order + dn = make_delivery_note(so.name) + dn.items[0].qty = 5 + dn.save() + + # Delivery note should have discount amount recalculated based on percentage + # and not affected by the repeated mapping logic + self.assertEqual(dn.additional_discount_percentage, 10) + self.assertEqual(dn.discount_amount, 50) # 10% of 500 -- GitLab From 48d91993cbf62d59265c8d9129d2185f97ec0500 Mon Sep 17 00:00:00 2001 From: Sagar Vora <16315650+sagarvora@users.noreply.github.com> Date: Fri, 17 Oct 2025 19:21:24 +0530 Subject: [PATCH 4/6] fix: handle returns as well --- erpnext/controllers/accounts_controller.py | 70 +++++++++++++------ .../tests/test_accounts_controller.py | 32 +++++++++ 2 files changed, 80 insertions(+), 22 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 74fa7692f2..4debf11610 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -3175,29 +3175,46 @@ class AccountsController(TransactionBase): return item_doctype = self.meta.get_field("items").options - item_meta = frappe.get_meta(item_doctype) + doctype_table = frappe.qb.DocType(self.doctype) + item_table = frappe.qb.DocType(item_doctype) - reference_fieldname = next( - ( - row.fieldname - for row in item_meta.fields - if row.fieldtype == "Link" - and row.options == source_doc.doctype - and not row.get("is_custom_field") - ), - None, - ) + is_same_doctype = self.doctype == source_doc.doctype + is_return = self.get("is_return") and is_same_doctype - if not reference_fieldname: + if is_same_doctype and not is_return: + # should never happen + # you don't map to the same doctype without it being a return return - doctype_table = frappe.qb.DocType(self.doctype) - item_table = frappe.qb.DocType(item_doctype) - discount_already_applied = ( + query = ( frappe.qb.from_(doctype_table) .where(doctype_table.docstatus == 1) .where(doctype_table.discount_amount != 0) - .where( + .select(Sum(doctype_table.discount_amount)) + ) + + if is_return: + query = query.where(doctype_table.is_return == 1).where( + doctype_table.return_against == source_doc.name + ) + + else: + item_meta = frappe.get_meta(item_doctype) + reference_fieldname = next( + ( + row.fieldname + for row in item_meta.fields + if row.fieldtype == "Link" + and row.options == source_doc.doctype + and not row.get("is_custom_field") + ), + None, + ) + + if not reference_fieldname: + return + + query = query.where( doctype_table.name.isin( frappe.qb.from_(item_table) .select(item_table.parent) @@ -3205,20 +3222,29 @@ class AccountsController(TransactionBase): .distinct() ) ) - .select(Sum(doctype_table.discount_amount)) - ).run() + result = query.run() + if not result: + return + + discount_already_applied = result[0][0] if not discount_already_applied: return - discount_already_applied = flt(discount_already_applied[0][0], self.precision("discount_amount")) + if is_return: + # returns have negative discount + discount_already_applied *= -1 + if (source_doc.discount_amount * (discount_already_applied - source_doc.discount_amount)) >= 0: # full discount already applied or exceeded self.discount_amount = 0 else: - self.discount_amount = flt( - self.discount_amount - discount_already_applied, self.precision("discount_amount") - ) + discount_amount = source_doc.discount_amount - discount_already_applied + if is_return: + # returns have negative discount + discount_amount *= -1 + + self.discount_amount = flt(discount_amount, self.precision("discount_amount")) self.calculate_taxes_and_totals() diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 2c86ab620e..eb469d38b3 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -2422,3 +2422,35 @@ class TestAccountsController(IntegrationTestCase): # and not affected by the repeated mapping logic self.assertEqual(dn.additional_discount_percentage, 10) self.assertEqual(dn.discount_amount, 50) # 10% of 500 + + def test_discount_amount_for_multiple_returns(self): + """ + Test that discount amount is correctly adjusted when multiple return invoices + are created against the same original invoice to prevent over-returning discount + """ + from erpnext.accounts.doctype.sales_invoice.sales_invoice import make_sales_return + + # Create original sales invoice with discount + si = create_sales_invoice(qty=10, rate=100, do_not_submit=True) + si.apply_discount_on = "Net Total" + si.discount_amount = 100 + si.save() + si.submit() + + # Create first return - Frappe will copy full discount by default, we need to adjust it + return_si_1 = make_sales_return(si.name) + return_si_1.items[0].qty = -6 # Return 6 out of 10 items + # Manually set discount to match the proportion (60% of discount) + return_si_1.discount_amount = -60 + return_si_1.save() + return_si_1.submit() + + self.assertEqual(return_si_1.discount_amount, -60) + + # Create second return for remaining items + return_si_2 = make_sales_return(si.name) + return_si_2.items[0].qty = -4 # Return remaining 4 out of 10 items + return_si_2.save() + + # Second return should only get remaining discount (100 - 60 = 40) + self.assertEqual(return_si_2.discount_amount, -40) -- GitLab From 855cbf29bfa4b3706c5612dc47f1ffc993683640 Mon Sep 17 00:00:00 2001 From: Sagar Vora <16315650+sagarvora@users.noreply.github.com> Date: Sat, 18 Oct 2025 00:00:36 +0530 Subject: [PATCH 5/6] refactor: simplify logic --- erpnext/controllers/accounts_controller.py | 13 ++++--------- erpnext/controllers/taxes_and_totals.py | 5 ++++- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 4debf11610..65eca8f8b9 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -3235,16 +3235,11 @@ class AccountsController(TransactionBase): # returns have negative discount discount_already_applied *= -1 - if (source_doc.discount_amount * (discount_already_applied - source_doc.discount_amount)) >= 0: - # full discount already applied or exceeded - self.discount_amount = 0 - else: - discount_amount = source_doc.discount_amount - discount_already_applied - if is_return: - # returns have negative discount - discount_amount *= -1 + discount_amount = max(source_doc.discount_amount - discount_already_applied, 0) + if discount_amount and is_return: + discount_amount *= -1 - self.discount_amount = flt(discount_amount, self.precision("discount_amount")) + self.discount_amount = flt(discount_amount, self.precision("discount_amount")) self.calculate_taxes_and_totals() diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index 5e6b689748..63f6ed7b92 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -728,7 +728,10 @@ class calculate_taxes_and_totals: grand_total = self.doc.grand_total # validate that discount amount cannot exceed the total before discount - if grand_total * (discount_amount - grand_total) > 0: + if ( + (grand_total >= 0 and discount_amount > grand_total) + or (grand_total < 0 and discount_amount < grand_total) # returns + ): frappe.throw( _( "Additional Discount Amount ({discount_amount}) cannot exceed " -- GitLab From 455081400bb6ef0f9445919b9945403962b05543 Mon Sep 17 00:00:00 2001 From: Sagar Vora <16315650+sagarvora@users.noreply.github.com> Date: Sat, 18 Oct 2025 00:08:38 +0530 Subject: [PATCH 6/6] chore: remove unused import --- erpnext/controllers/taxes_and_totals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index 63f6ed7b92..614a56797b 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -7,7 +7,7 @@ import json import frappe from frappe import _, scrub from frappe.model.document import Document -from frappe.utils import cint, flt, fmt_money, round_based_on_smallest_currency_fraction +from frappe.utils import cint, flt, round_based_on_smallest_currency_fraction import erpnext from erpnext.accounts.doctype.journal_entry.journal_entry import get_exchange_rate -- GitLab