From 44539f094469003c1663ee202c3a1618a655e6d9 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/7] fix: ensure that additional discount amount is not mapped repeatedly (cherry picked from commit feb62102d95868756108665a75f6130150a8f837) --- 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 6410c28ed19..3a1f4c718b0 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -39,6 +39,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, @@ -2918,6 +2920,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): From e559fafa83359d5d2e2240ffd4c50f35368af2c4 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/7] fix: validate that discount amount cannot exceed total before discount (cherry picked from commit f4f79d99e45a4e1f785d26713bf9f00045e9571f) # Conflicts: # erpnext/controllers/taxes_and_totals.py --- erpnext/controllers/taxes_and_totals.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index 5543129d323..0048ec9acab 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -7,8 +7,12 @@ import json import frappe from frappe import _, scrub from frappe.model.document import Document +<<<<<<< HEAD from frappe.utils import cint, flt, round_based_on_smallest_currency_fraction from frappe.utils.deprecations import deprecated +======= +from frappe.utils import cint, flt, fmt_money, round_based_on_smallest_currency_fraction +>>>>>>> f4f79d99e4 (fix: validate that discount amount cannot exceed total before discount) import erpnext from erpnext.accounts.doctype.journal_entry.journal_entry import get_exchange_rate @@ -682,6 +686,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: From 22b6760164e0cbf1121929d69e07281146d1c7f8 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/7] test: some tests to ensure correct discount mapping (cherry picked from commit 0968f435d28dd3f6aab9bd6ccbeb8210670b95bd) --- .../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 f2558572dc9..339607e9774 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -2258,3 +2258,141 @@ class TestAccountsController(FrappeTestCase): 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 From 9ed40cc17d1e353da351f1b73a6fd251cc31d0b0 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/7] fix: handle returns as well (cherry picked from commit 0e026b9ccd93e4e502cf55b177ee76f6edbc8440) --- erpnext/controllers/accounts_controller.py | 74 +++++++++++++------ .../tests/test_accounts_controller.py | 32 ++++++++ 2 files changed, 82 insertions(+), 24 deletions(-) diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 3a1f4c718b0..01b10e460c1 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -2950,29 +2950,46 @@ class AccountsController(TransactionBase): 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 = ( + + is_same_doctype = self.doctype == source_doc.doctype + is_return = self.get("is_return") and is_same_doctype + + 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 + + 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) @@ -2980,20 +2997,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 339607e9774..899ec45c169 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -2396,3 +2396,35 @@ class TestAccountsController(FrappeTestCase): # 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) From b4df87e545143f8e21eea41bdc80224bdf60cb8f 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/7] refactor: simplify logic (cherry picked from commit 95f604457d42e601d39108a658d36cbe8c214df6) --- 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 01b10e460c1..f60fe003b0d 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -3010,16 +3010,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 0048ec9acab..3aa2a88a17c 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -690,7 +690,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 " From 22848eb4da623e8943dad5963e184fe85e4a4bb1 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/7] chore: remove unused import (cherry picked from commit 81ab15351e88352aeae9db0d9adb0db0f13bafc6) # Conflicts: # erpnext/controllers/taxes_and_totals.py --- erpnext/controllers/taxes_and_totals.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index 3aa2a88a17c..50bae1378b5 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -8,11 +8,15 @@ import frappe from frappe import _, scrub from frappe.model.document import Document <<<<<<< HEAD +<<<<<<< HEAD from frappe.utils import cint, flt, round_based_on_smallest_currency_fraction from frappe.utils.deprecations import deprecated ======= from frappe.utils import cint, flt, fmt_money, round_based_on_smallest_currency_fraction >>>>>>> f4f79d99e4 (fix: validate that discount amount cannot exceed total before discount) +======= +from frappe.utils import cint, flt, round_based_on_smallest_currency_fraction +>>>>>>> 81ab15351e (chore: remove unused import) import erpnext from erpnext.accounts.doctype.journal_entry.journal_entry import get_exchange_rate From 313e6af52853ac2123a37a018319608ab7ec2ba1 Mon Sep 17 00:00:00 2001 From: Sagar Vora <16315650+sagarvora@users.noreply.github.com> Date: Tue, 11 Nov 2025 15:11:52 +0530 Subject: [PATCH 7/7] chore: resolve conflicts --- erpnext/controllers/taxes_and_totals.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index 50bae1378b5..eda94351515 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -7,16 +7,8 @@ import json import frappe from frappe import _, scrub from frappe.model.document import Document -<<<<<<< HEAD -<<<<<<< HEAD from frappe.utils import cint, flt, round_based_on_smallest_currency_fraction from frappe.utils.deprecations import deprecated -======= -from frappe.utils import cint, flt, fmt_money, round_based_on_smallest_currency_fraction ->>>>>>> f4f79d99e4 (fix: validate that discount amount cannot exceed total before discount) -======= -from frappe.utils import cint, flt, round_based_on_smallest_currency_fraction ->>>>>>> 81ab15351e (chore: remove unused import) import erpnext from erpnext.accounts.doctype.journal_entry.journal_entry import get_exchange_rate