diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 5d4630d7d5aa9e281e3b4b01415b1c18a7a2cb2a..65eca8f8b921b91a6e2c29e0b3698b0aa4e7f454 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,104 @@ 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 + doctype_table = frappe.qb.DocType(self.doctype) + item_table = frappe.qb.DocType(item_doctype) + + 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) + .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) + .where(item_table[reference_fieldname] == source_doc.name) + .distinct() + ) + ) + + result = query.run() + if not result: + return + + discount_already_applied = result[0][0] + if not discount_already_applied: + return + + if is_return: + # returns have negative discount + discount_already_applied *= -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.calculate_taxes_and_totals() + @frappe.whitelist() def get_tax_rate(account_head): diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index d3c73387f9d0c02b76019d97e2611b7fe108d591..614a56797b30eab8b71ada62cd090a7765a86f77 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -724,6 +724,25 @@ 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 >= 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 " + "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: diff --git a/erpnext/controllers/tests/test_accounts_controller.py b/erpnext/controllers/tests/test_accounts_controller.py index 706268a662e4e5b984a05d4848e39a1dbcb80a9e..eb469d38b3827219d0eaea29e3cb5f7a4479ff53 100644 --- a/erpnext/controllers/tests/test_accounts_controller.py +++ b/erpnext/controllers/tests/test_accounts_controller.py @@ -2284,3 +2284,173 @@ 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 + + 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)